Matthew Wilcox wrote:
> On Sun, Jul 07, 2002 at 04:45:21PM -0700, Dave Hansen wrote:
>
>>Don't forget that the BKL is released on sleep. It is OK to hold it
>>over a schedule()able operation. If I remember right, there is no
>>real protection needed for the file->private_data either because there
>>is 1 and only 1 struct file per open, and the data is not shared among
>>struct files.
>
> one struct file per open(), yes. however, fork() shares a struct file,
> as does unix domain fd passing. so we need protection between different
> processes. there's some pretty good reasons to want to use a semaphore
> to protect the struct file (see fasync code.. bleugh).
But, this at least means that we don't need to protect
file->private_data during the open itself, right?
> however, our semaphores currently suck. they attempt to acquire the sem
> immediately and if they fail go straight to sleep. schimmel (i think..)
> suggests spinning for a certain number of iterations before sleeping.
> the great thing is, it's all out of line slowpath code so the additional
> size shouldn't matter. obviously this is SMP-only, and it does require
> someone to do it who can measure the difference (and figure out how may
> iterations to spin for before sleeping).
Well, I certainly have the hardware to measure the difference. But, I
seem to remember several conversations in the past where people didn't
like this behavior.
http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&oe=UTF-8&safe=off&threadm=linux.kernel.3C62DABA.3020906%40us.ibm.com
> i was wondering if this might be a project you'd like to take on which
> would upset far fewer people and perhaps yield greater advantage.
Yes, something less controvertial, please! A dumb implementation
would be pretty easy on top of current semaphores, but I think it was
already done (see above).
--
Dave Hansen
[email protected]
On Sun, 7 Jul 2002, Dave Hansen wrote:
> > as does unix domain fd passing. so we need protection between different
> > processes. there's some pretty good reasons to want to use a semaphore
> > to protect the struct file (see fasync code.. bleugh).
>
> But, this at least means that we don't need to protect
> file->private_data during the open itself, right?
Correct. Moreover, most of the struct file instances that have ->private_data
tend to set it during ->open() and never change it afterwards.
> > however, our semaphores currently suck. they attempt to acquire the sem
> > immediately and if they fail go straight to sleep. schimmel (i think..)
> > suggests spinning for a certain number of iterations before sleeping.
> > the great thing is, it's all out of line slowpath code so the additional
> > size shouldn't matter. obviously this is SMP-only, and it does require
> > someone to do it who can measure the difference (and figure out how may
> > iterations to spin for before sleeping).
The thing being, if you are already contended you are playing "I'll release
CPU now" vs. "I'll spin in hope that contender will go away right now".
IOW, it's a win only if you get contention often and for short intervals.
Which is a very good indication that something is rotten with your locking
scheme. Like, say it, having lost the control over the amount of locks
as the result of brainde^Woverenthusiastic belief that fine-grained ==
good. With everything that follows from that...
On Sun, Jul 07, 2002 at 07:52:34PM -0700, Dave Hansen wrote:
> Well, I certainly have the hardware to measure the difference. But, I
> seem to remember several conversations in the past where people didn't
> like this behavior.
> http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&oe=UTF-8&safe=off&threadm=linux.kernel.3C62DABA.3020906%40us.ibm.com
I think this is a very different philosophy. That's "well, this should
be a spinlock, but sometimes we sleep, and we don't want to think about
it too hard, so let's invent a magical lock". This is "sometimes we
don't hold semaphores for very long so we can improve performance by
spinning 1000 times before sleeping".
--
Revolutions do not require corporate support.
On Sun, Jul 07, 2002 at 11:06:32PM -0400, Alexander Viro wrote:
> The thing being, if you are already contended you are playing "I'll release
> CPU now" vs. "I'll spin in hope that contender will go away right now".
>
> IOW, it's a win only if you get contention often and for short intervals.
> Which is a very good indication that something is rotten with your locking
> scheme. Like, say it, having lost the control over the amount of locks
> as the result of brainde^Woverenthusiastic belief that fine-grained ==
> good. With everything that follows from that...
So let's get some numbers. It really shouldn't be hard to make our
current semaphores spin a little before they sleep. If we get some
numbers showing it does help then either we need this change in mainline
or we need to fix our locking.
--
Revolutions do not require corporate support.