Messages in this thread | | | Date | Tue, 23 Oct 2012 17:12:24 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/2] brw_mutex: big read-write mutex |
| |
Hi Mikulas,
On 10/22, Mikulas Patocka wrote: > > On Fri, 19 Oct 2012, Oleg Nesterov wrote: > > > On 10/19, Mikulas Patocka wrote: > > > > > > synchronize_rcu() is way slower than msleep(1) - > > > > This depends, I guess. but this doesn't mmatter, > > > > > so I don't see a reason > > > why should it be complicated to avoid msleep(1). > > > > I don't think this really needs complications. Please look at this > > patch for example. Or initial (single writer) version below. It is > > not finished and lacks the barriers too, but I do not think it is ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
please note the comment above ;)
> > more complex. > > Hi > > My implementation has a smaller structure (it doesn't have > wait_queue_head_t).
Oh, I don't think sizeof() really matters in this case.
> Your implementation is prone to starvation - if the writer has a high > priority and if it is doing back-to-back write unlocks/locks, it may > happen that the readers have no chance to run.
Yes, it is write-biased, this was intent. writers should be rare.
> The use of mutex instead of a wait queue in my implementation is unusual, > but I don't see anything wrong with it
Neither me.
Mikulas, apart from _rcu/_sched change, my only point was msleep() can (and imho should) be avoided.
> > static inline long brw_read_ctr(struct brw_sem *brw) > > { > > long sum = 0; > > int cpu; > > > > for_each_possible_cpu(cpu) > > sum += per_cpu(*brw->read_ctr, cpu); > > Integer overflow on signed types is undefined - you should use unsigned > long - you can use -fwrapv option to gcc to make signed overflow defined, > but Linux doesn't use it.
I don't think -fwrapv can make any difference in this case, but I agree that "unsigned long" makes more sense.
> > void brw_up_write(struct brw_sem *brw) > > { > > brw->writer = NULL; > > synchronize_sched(); > > That synchronize_sched should be put before brw->writer = NULL.
Yes, I know. I mentioned this at the start, this lacks the necessary barrier between this writer and the next reader.
> I had this bug in my implementation too.
Yes, exactly. And this is why I cc'ed you initially ;)
Oleg.
| |