Date:   Tue, 04 Aug 2020 08:21:51 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Christian Brauner <christian.brauner@...ntu.com>
Cc:     Kirill Tkhai <ktkhai@...tuozzo.com>, akpm@...ux-foundation.org,
        viro@...iv.linux.org.uk, adobriyan@...il.com, davem@...emloft.net,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/8] namespaces: Introduce generic refcount

Christian Brauner <christian.brauner@...ntu.com> writes:

> On Tue, Aug 04, 2020 at 07:11:59AM -0500, Eric W. Biederman wrote:
>> Christian Brauner <christian.brauner@...ntu.com> writes:
>> 
>> > On Mon, Aug 03, 2020 at 01:16:10PM +0300, Kirill Tkhai wrote:
>> >> Every namespace type has its own counter. Some of them are
>> >> of refcount_t, some of them are of kref.
>> >> 
>> >> This patchset introduces generic ns_common::count for any
>> >> type of namespaces instead of them.
>> >> 
>> >> ---
>> >
>> > I was wondering why that series never made it to me turns out there's
>> > some weird bug in my (neo)mutt where it sometimes marks messages as read
>> > when I'm deleting completely unrelated messages. That has already cost
>> > me a talk slot for an event I really wanted to attend and now it seems
>> > to start costing me patches... I need to figure this out.
>> >
>> > Anyway, thanks for sending this. I pulled this into my tree now.
>> 
>> Actually why in the world should the reference count be generic?
>> 
>> What is the point of this patchset?
>> 
>> What problem does it solve.  Name spaces are not the same, and
>> their refcounting needs are not the same so I don't have a clue how it
>> helps anything to have a reference count in ns_common.
>
> What is the point of this opposition to this cleanup?
>
> It unifies reference counting across namespaces and gets rid of
> inconsistencices. Over the years none of the namespaces seem to have
> deviated enough from each that they really have needed separate
> reference counting mechanisms.

First this posting is the first I have seen of it, unless it was a
subset of the weird /proc/namespaces/ patchset that has design problems.
In which case I never got this far.

Second I don't see a motivation for this.  The only point to place a
reference count in ns_common is if it makes something easier.  What
does it make easier and what does it make harder?

For a pure cleanup the questions are what are the trade offs.
There are potential performance differences between refcount_t and
kfref.

>From a practical matter it makes absolutely no sense in the least to
talk about the reference count, when some of the namespaces have more
than one reference count, with difference semantics and they interrelate
in somewhat subtle ways.

Further depending on what is happening sharing code that does not
have a fundamental reason to be shared, can make maintenance more
difficult as the entire generic infrastructure will need to be updated
instead of just that the part that focuses on the one thing.

So I am opposed because the patchset does not explain at all why it
makes sense to do, nor what tradeoffs were considered, nor what
testing was done.

This change is not as trivial as a spelling change so it is not ok to
say it is just a cleanup and move on.  A change in the reference
counting can be noticable.  This needs at least to be acknowledged in
the change log and at a minimum a hand wavy reason put forth why it is
ok.


Instead what I am seeing as justification is this is a trivial cleanup
and no one will notice or care.   And it is not that trivial so I
object to the patchset.

Eric

Powered by blists - more mailing lists