Greg KH <[email protected]> writes:
> On Mon, Jan 30, 2006 at 01:13:12PM -0700, Eric W. Biederman wrote:
>
> What function would you have to add? A release function? You should
> have that logic somewhere already, so net gain should be the same.
Yes inline in my if statement all 3 lines of it. Adding a function
makes the code less clear.
>> The extra debugging checks in krefs are nice, but not something I
>> am lusting over.
>
> Heh, but they do catch a lot of improper usages.
Agreed.
>> As for code recognition I don't see how recognizing the atomic_t idiom
>> versus the kref idiom is much different. But I haven't had this issue
>> come up in a code review so I have no practical experience there.
>
> It's not a different "idiom", but rather, a way to implement the
> reference counting logic without having to code it all by hand, and
> ensure that you get it correct.
Yes a way to implement the reference counting logic that requires more
lines of code to implement and obscures my code in this instance.
> They are used all the time, and there's not that many atomic_t usages in
> the kernel that do reference counting that have not been already
> converted to use kref (the vfs not-withstanding, for other reasons.)
Well either that is all code that is still to be merged in the
mainline or I am looking with a very different filter than you are.
A quick grep for atomic_t in the kernel include files (filtering
out the asm headers where it was defined) I got several screen fulls
of hits. Things like the network stack, shared process resources are
places I have worked lately and where atomic_t is used, all of the
core bits of the kernel. My similar grep for kref should barely returned
a screen full of hits.
>> So those are all of the nits I can pick. :) I don't kref seems to be
>> a perfectly usable piece of code but not one that seems to help my
>> code. Unless it becomes a mandate from the code correctness police.
>
> As you are adding a new reference count, it is highly encouraged that
> you not reimplement the same logic, but rather use the functions already
> provided in the kernel to do that work for you. That way you don't have
> to write the extra code, and we don't have to check the logic for you.
If there was something to implement I would agree with you. In drivers
out at the fringes of the kernel where looking at some of the code submissions
you wonder if the implementors know C I can totally understand anything
to make things less error prone. But that isn't what this code is.
Given how short my code is and what the effects are I am actually
disappointed that the review only got as far as noticing I was using
an atomic_t. Oh well. Thanks for getting that far.
Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
