(x | y) - y pourquoi ne peut-il pas simplement être x ou même `x | 0`

47

Je lisais un code du noyau, et à un endroit, j'ai vu une expression à l'intérieur d'une ifdéclaration comme

if (value == (SPINLOCK_SHARED | 1) - 1) {
         ............
}

SPINLOCK_SHARED = 0x80000000est une constante prédéfinie.

Je me demande pourquoi avons-nous besoin (SPINLOCK_SHARED | 1) - 1- à des fins de conversion de type? le résultat de l'expression serait 80000000-- identique à 0x80000000, n'est-ce pas? pourtant, pourquoi ORing 1 et Soustraire 1 sont importants?

J'ai le sentiment que je manque pour obtenir quelque chose ..

RaGa__M
la source
3
#define SPINLOCK_SHARED 0x80000000
RaGa__M
1
Je soupçonne qu'il n'y a aucune raison. Peut-être un copier-coller. Pourriez-vous ajouter où exactement vous avez trouvé cela (quelle version de quel noyau, quel fichier, etc.).
Sander De Dycker
2
Le même fichier de code source contient également if (atomic_cmpset_int(&spin->counta, SPINLOCK_SHARED|0, 1)).
Eric Postpischil
2
Ensuite, je pense que nous devons demander à l'auteur pourquoi il a été modifié.
funnydman

Réponses:

1

Cela a été fait de cette façon pour plus de clarté, c'est tout. C'est parce que atomic_fetchadd_int () (par exemple sys / spinlock2.h) renvoie la valeur PRIOR à l'addition / soustraction, et cette valeur est passée à _spin_lock_contested ()

Notez que le compilateur C pré-calcule complètement toutes les expressions constantes. En fait, le compilateur peut même optimiser le code en ligne basé sur des conditions qui utilisent des arguments de procédure transmis lorsque les procédures reçoivent des constantes dans ces arguments. C'est pourquoi l'inline lockmgr () dans sys / lock.h a une instruction case .... car cette instruction case entière sera optimisée et transformée en un appel direct à la fonction appropriée.

De plus, dans toutes ces fonctions de verrouillage, les frais généraux des opérations atomiques éclipsent tous les autres calculs de deux ou trois ordres de grandeur.

-Mat

Matthew Dillon
la source
Cette réponse est de l'auteur !!!
RaGa__M
31

Le code se trouve dans _spin_lock_contested, qui est appelé à partir du _spin_lock_quickmoment où quelqu'un d'autre tente d'obtenir le verrou:

count = atomic_fetchadd_int(&spin->counta, 1);
if (__predict_false(count != 0)) {
    _spin_lock_contested(spin, ident, count);
}

S'il n'y a pas de concours, alors count(la valeur précédente) devrait l'être 0, mais ce n'est pas le cas. Cette countvaleur est transmise en tant que paramètre à en _spin_lock_contestedtant que valueparamètre. Ceci valueest ensuite vérifié avec ifl'OP:

/*
 * WARNING! Caller has already incremented the lock.  We must
 *      increment the count value (from the inline's fetch-add)
 *      to match.
 *
 * Handle the degenerate case where the spinlock is flagged SHARED
 * with only our reference.  We can convert it to EXCLUSIVE.
 */
if (value == (SPINLOCK_SHARED | 1) - 1) {
    if (atomic_cmpset_int(&spin->counta, SPINLOCK_SHARED | 1, 1))
        return;
}

En gardant à l'esprit que valuec'est la valeur précédente de spin->counta, et que celle-ci a déjà été incrémentée de 1, nous nous attendons spin->countaà égaler value + 1(sauf si quelque chose a changé entre-temps).

Donc, vérifier si spin->counta == SPINLOCK_SHARED | 1(la condition préalable de atomic_cmpset_int) correspond à vérifier si value + 1 == SPINLOCK_SHARED | 1, qui peut être réécrit comme value == (SPINLOCK_SHARED | 1) - 1(encore une fois, si rien n'a changé entre-temps).

Bien qu'il value == (SPINLOCK_SHARED | 1) - 1puisse être réécrit tel value == SPINLOCK_SHAREDquel, il reste tel quel, pour clarifier l'intention de la comparaison (c'est-à-dire comparer la valeur précédente incrémentée avec la valeur de test).

Ou iow. la réponse semble être: pour la clarté et la cohérence du code.

Sander De Dycker
la source
Merci pour votre réponse, tout sauf une (SPINLOCK_SHARED | 1) - 1partie est compréhensible et value == SPINLOCK_SHAREDc'est ma pensée aussi, car nous vérifions si la valeur précédente a un indicateur partagé.si oui, transformez le verrou en exclusif .........
RaGa__M
1
@RaGa__M: le but de la ifvérification est de vérifier si value + 1(ce qui devrait être la même valeur que spin->countasi rien n'a changé entre-temps) est égal SPINLOCK_SHARED | 1. Si vous écrivez le ifchèque en tant que value == SPINLOCK_SHARED, cette intention n'est pas claire et il serait beaucoup plus difficile de comprendre ce que signifie le chèque. Garder les deux SPINLOCK_SHARED | 1et - 1explicitement dans le ifchèque est intentionnel.
Sander De Dycker
Mais cela crée en fait de la confusion.
RaGa__M
Pourquoi ne pas if (value + 1 == (SPINLOCK_SHARED | 1) )?
Pablo H
Eh bien ... cela pourrait tout simplement être value & SPINLOCK_SHAREDplus lisible.
RaGa__M
10

Je pense que l'objectif est probablement d'ignorer le bit significatif le plus bas:

  • Si SPINLOCK_SHARED exprimé en binaire est xxx0 -> le résultat est xxx0
  • Si SPINLOCK_SHARED = xxx1 -> le résultat est également xxx0

aurait peut-être été plus clair d'utiliser une expression de masque de bits?

Guillaume Petitjean
la source
8
C'est ce que fait le code, mais la question est de savoir pourquoi le feriez-vous pour une constante définie qui n'a pas le bit le moins significatif défini?
Sander De Dycker
4
@SanderDeDycker Parce que le noyau Linux?
Lundin
9
@Lundin Le noyau linux n'est pas exempt de pratiques de codage compréhensibles. Plutôt l'inverse.
Qix - MONICA A ÉTÉ BRUÉE
2
@Qix Si vous le dites. J'étais un grand fan de Linux jusqu'à ce que je jette un œil au code et lise le document de style de codage du noyau. Aujourd'hui, je garde une distance de sécurité de 10 mètres par rapport aux ordinateurs Linux.
Lundin
2
@Qix Nah, je le juge plutôt sur la base de son code source ...
Lundin
4

L'effet de

(SPINLOCK_SHARED | 1) - 1

est de s'assurer que le bit de poids faible du résultat est effacé avant la comparaison avec value. Je suis d'accord qu'il semble plutôt inutile, mais apparemment le bit de poids faible a une utilisation ou une signification particulière qui n'est pas apparente dans ce code, et je pense que nous devons supposer que les développeurs avaient une bonne raison de le faire. Une question intéressante serait - ce même modèle ( | 1) -1) est-il utilisé dans toute la base de code que vous regardez?

Bob Jarvis - Réintégrer Monica
la source
2

C'est une façon obscure d'écrire un petit masque. Version Lisible: value == (SPINLOCK_SHARED & ~1u).

Lundin
la source
5
Oui, mais pourquoi . Le PO demande pourquoi ce serait le cas s'il SPINLOCK_SHAREDs'agit d'une constante connue. S'ils testent simplement SPINLOCK_SHAREDleur présence dans un masque, pourquoi pas if (value & SPINLOCK_SHARED)?
Qix - MONICA A ÉTÉ BRUÉE
4
value == (SPINLOCK_SHARED & ~1u)n'est pas équivalent car value == (SPINLOCK_SHARED | 1) - 1fonctionne même si le type de SPINLOCK_SHAREDest plus large que unsigned.
Eric Postpischil
4
Honnêtement, je ne suis pas sûr que ce & ~1usoit plus clair. J'ai pensé à suggérer & 0xFFFFFFFEdans ma réponse mais j'ai réalisé que ce n'était pas vraiment clair non plus. Votre suggestion a cependant l'avantage d'être concise. :-)
Bob Jarvis - Réintègre Monica
6
@Lundin: Nous ne savons pas que ce sera 0x80000000 . OP a déclaré qu'il est défini avec #define SPINLOCK_SHARED 0x80000000, mais cela pourrait être à l'intérieur #if…#endifet une définition différente est utilisée dans d'autres circonstances, ou l'auteur de ce code aurait pu le faire fonctionner même si la définition est modifiée ou le code est compilé avec d'autres en-têtes qui le définir différemment. Quoi qu'il en soit, les deux morceaux de code ne sont pas équivalents en eux-mêmes.
Eric Postpischil
2
@ BobJarvis-ReinstateMonica C'est beaucoup plus clair pour les personnes qui travaillent quotidiennement avec des opérateurs au niveau du bit. Le mélange au niveau du bit avec l'arithmétique régulière est source de confusion.
Lundin
0

La plupart comme cela est fait pour gérer plusieurs cas supplémentaires. Par exemple, dans ce cas, nous disons que SPINLOCK_SHAREDne peut pas être 1:

int SPINLOCK_SHARED = 0x01

int res = (SPINLOCK_SHARED | 1) - 1 // 0
funnydman
la source
2
Cela aurait du sens mais, même si cela ne ressort pas clairement de la question, cela ressemble à SPINLOCK_SHAREDune constante définie et à la variable testée value. Dans ce cas, le mystère demeure.
Roberto Caboni
Merci pour votre réponse, mais je pense que dans le cas d'origine, SPINLOCK_SHARED n'est pas 0x01, vous avez gardé la | 1) - 1partie, alors que SPINLOCK_SHARED tient 0x80000000quel serait l'impact de | 1) - 1?
RaGa__M
La seule raison pour laquelle je pouvais penser, c'est qu'ils voulaient éviter de SPINLOCK_SHAREDchanger à l'avenir. Mais ce n'est pas du tout clair. J'écrirais aux développeurs du noyau et demanderais qu'un commentaire de clarification soit fait ou que l'expression soit réorganisée pour qu'elle se documente d'elle-même.
Qix - MONICA A ÉTÉ BRUÉE