On 29.05.2024 09:50, Oleksii K. wrote: > On Tue, 2024-05-28 at 09:53 +0100, Julien Grall wrote: >>>>> +/** >>>>> + * generic_test_bit - Determine whether a bit is set >>>>> + * @nr: bit number to test >>>>> + * @addr: Address to start counting from >>>>> + * >>>>> + * This operation is non-atomic and can be reordered. >>>>> + * If two examples of this operation race, one can appear to >>>>> succeed >>>>> + * but actually fail. You must protect multiple accesses with >>>>> a >>>>> lock. >>>>> + */ >>>> >>>> You got carried away updating comments - there's no raciness for >>>> simple test_bit(). As is also expressed by its name not having >>>> those >>>> double underscores that the others have. >>> Then it is true for every function in this header. Based on the >>> naming >>> the conclusion can be done if it is atomic/npn-atomic and can/can't >>> be >>> reordered. >> >> So let me start with that my only request is to keep the existing >> comments as you move it. It looks like there were none of test_bit() >> before. > Just to clarify that I understand correctly. > > Do we need any comment above functions generic_*()? Based on that they > are implemented in generic way they will be always "non-atomic and can > be reordered.".
I indicated before that I think reproducing the same comments __test_and_* already have also for generic_* isn't overly useful. If someone insisted on them being there as well, I could live with that, though. > Do you find the following comment useful? > > " * If two examples of this operation race, one can appear to succeed > * but actually fail. You must protect multiple accesses with a lock." > > It seems to me that it can dropped as basically "non-atomic and can be > reordered." means that. I agree, or else - as indicated before - the wording would need to further change. Yet iirc you've added that in response to a comment from Julien, so you'll primarily want his input as to the presence of something along these lines. Jan
