Re: Found it! (was Re: [3.10] Oopses in kmem_cache_allocate() viaprepare_creds())

From: Ingo Molnar
Date: Mon Dec 02 2013 - 12:05:24 EST



* Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:

> On Mon, Dec 02, 2013 at 05:27:55PM +0100, Ingo Molnar wrote:
>
> > It's not like there should be many (any?) VFS operations where a pipe
> > is used via i_mutex and pipe->mutex in parallel, which would improve
> > scalability - so I don't see the scalability advantage. (But I might
> > be missing something)
> >
> > Barring such kind of workload the extra mutex just adds extra
> > micro-costs because now two locks have to be taken on
> > creation/destruction, plus it adds extra complexity and races.
> >
> > So unless I'm missing something obvious, another good fix would be to
> > just revert pipe->mutex and rely on i_mutex as before?
>
> You are missing the extra shitloads of complexity in ->i_mutex ordering,
> and ->i_mutex is already used for too many things...

Well, AFAICS the split-out did not reduce ordering complexity but
increased it, at least in the short term: pipe->mutex now has to be
taken in the right order with i_mutex, the subject of the bug here.

Plus AFAICS where i_mutex was used for pipe-internal purposes we used
pretty generic facilities like user-copy, signal-sending, wakeups,
etc. - none of which is really adding complexity to i_mutex ordering,
as those are always expected to be facilities independent of the VFS
in the future as well.

Anyway, it's your call obviously.

In any case, what prompted my reply was the overly terse nature of the
changelog, would it make sense to put more verbose reasoning into
changelogs, especially where such a change has a seemingly non-obvious
(to me) cost/benefit balance?

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/