On Tue, 2018-02-20 at 11:14 -0500, Doug Ledford wrote:
> On Tue, 2018-02-20 at 07:45 +0200, Leon Romanovsky wrote:
> >
> > Sometimes, it makes sense to provide wrappers e.g. different get/put,
> > to_..(), from_..()) calls for annotate ambiguous ++/--. But locks
> > are different, they need to be clear and can't be hided.
> >
> > So yes, I'm objecting here for this lock/unlock wrapper and would like
> > to see more pushback from maintainers on extensive usage of ridiculous
> > wrappers (including mlx code).
>
> I've spent some time thinking about this, and I'm not sure I agree with
> you Leon. Getting locking right is important, and in this case it
> doesn't really hide any details, but it gets all of the annotations
> right. I'm personally inclined to take the patch. I'll look around at
> some other places in the kernel, but wrappers for getting locking right
> aren't that rare I don't think.
I used the following command to do a quick scan of whether or not
wrappers for locking are common:
grep -r __acquires -B 2 --include=*.[hc]
After reviewing the output, I'm fine with this patch. It shows that
some authors do simple locking directly in the functions that need to
take the lock, but by in large, when either the locking complexity goes
up or even just the annotation complexity goes up, locking helpers are
in fact very common. As an example, kernel/sched/sched.h is full of
helpers.
I'm also going to disagree with you on the "more pushback". It is going
to depend on the complexity of the locking and annotations as to whether
I might push back.
--
Doug Ledford <dledford@xxxxxxxxxx>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDDAttachment:
signature.asc
Description: This is a digitally signed message part
