linux-kernel - Re: [PATCH net 1/3] kref: add kref_sub

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOMFOmWwtQL4TXhWgXoOef3P+o0GL=cSdbdMZ449_A9CdndLbQ@mail.gmail.com>
Date:	Wed, 12 Feb 2014 10:39:17 -0800
From:	Anatol Pomozov <anatol.pomozov@...il.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Jason Wang <jasowang@...hat.com>,
	virtio-dev@...ts.oasis-open.org,
	virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org,
	davem@...emloft.net, qinchuanyu@...wei.com,
	Joern Engel <joern@...fs.org>,
	Nicholas Bellinger <nab@...ux-iscsi.org>
Subject: Re: [PATCH net 1/3] kref: add kref_sub_return

Hi

On Wed, Feb 12, 2014 at 9:35 AM, Michael S. Tsirkin <mst@...hat.com> wrote:
> On Wed, Feb 12, 2014 at 08:56:30AM -0800, Greg Kroah-Hartman wrote:
>> On Wed, Feb 12, 2014 at 06:38:21PM +0200, Michael S. Tsirkin wrote:
>> > It is sometimes useful to get the value of the reference count after
>> > decrement.
>> > For example, vhost wants to execute some periodic cleanup operations
>> > once number of references drops below a specific value, before it
>> > reaches zero (for efficiency).
>>
>> You should never care about what the value of the kref is, if you are
>> using it correctly :)
>>
>> So I really don't want to add this function, as I'm sure people will use
>> it incorrectly.  You should only care if the reference drops to 0, if
>> not, then your usage doesn't really fit into the "kref" model, and so,
>> just use an atomic variable.
>
> This happens when you have code that keeps
> reference itself implicitly or explicitly.
>
>         foo(struct kref *k, int bar) {
>
>         sub = kref_sub(k)
>
>         if (sub == 1)

At this moment you cannot be sure that refcount is 1. It might be
changed between kref_sub call and this point. The refcount might be 0,
1, ... or any other value.

The only value that you can be sure is zero. Once refcount becomes
zero there is no way to increase it to positive value as there are no
alive pointers to the object.

>                 FOO(k, bar) /* Here I am the only one
>                                with a reference */
>
>         }
>
>         kref_get(k)
>         foo(k, bar);
>         ....
>         kref_put(k)
>
> Why not do FOO in destructor you ask?
> Absolutely but this will be called much later.
>
> Maybe you will reconsider if I document this
> as the only legal use?
>
>>
>> I really want to know why it matters for "efficiency" that you know this
>> number.  How does that help anything, as the number could then go up
>> later on, and the work you did at a "lower" number is obsolete, right?
>>
>> thanks,
>>
>> greg k-h
>
> The issue is that if number dropped to 1, this means
> we must do the cleanup work since there are
> no outstanding buffers, (last user is ourselves)
> if we do not cleanup,
> guest will hang waiting for us.
>
> But it never drops to 0 since we have our own reference
> in the device.
> If it goes up again this means we didn't have
> to do cleanup, but an alternative is doing
> it all the time and that is slow.
>
> Yes I can rework vhost to open-code this kref use, it's
> no big deal.
> Alternatively since most of the use does match kref
> model, maybe __kref_sub_return with disclaimers
> that you must know what you are doing?
> Please let me know.
>
> Thanks!
>
> --
> MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/