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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/