On Tue, 2013-09-03 at 18:27 -0700, Andrew Morton wrote: > On Tue, 03 Sep 2013 09:06:58 -0700 "Nicholas A. Bellinger" > <n...@linux-iscsi.org> wrote: > > > Are there any other review comments to be addressed for this patch..? > > > > If not, please kindly give your Reviewed-by if your OK for an initial > > standalone merge. > > Sorry, I'm largely offline for the next week... > > It looks OK to me (that's as close as I get to an ack :)) >
Thanks. That quote is going in the commit log btw. ;) > Nit: > > > + if (cpu >= nr_cpu_ids) > > + cpu = cpumask_first(&pool->cpus_have_tags); > > + > > + if (cpu >= nr_cpu_ids) > > + BUG(); > > The second `if' can be moved inside the first, but hopefully the > compiler already did that. Fixed. > > I have dim memories that doing > > local_irq_save() > spin_lock() > > is not equivalent to > > spin_lock_irqsave() > > in some circumstances. Perhaps a lockdep thing. Or perhaps this was fixed in > the intervening years and this is no longer the case. Mmmm, I had wondered the same thing on the release side with kref_put_spinlock_irqsave() here before: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/include/linux/kref.h?id=ccf5ae83a6cf3d9cfe9a7038bfe7cd38ab03d5e1 but at some point was convinced it was not a issue anymore.. > > Such things probably won't be detected during 3.12-rcX because few > people will test iscsi, I expect. So please be sure that you've > runtime tested it with all the lockdeppy things enabled. > Documentation/SubmitChecklist section 12 has a list, but it might be > dated. <nod> FYI, my v3.11-rc5 kernel has been running with CONFIG_LOCKDEP_SUPPORT=y using vhost/iscsi/iser fabrics on the -v5 patch, and not generated any warnings (yet). I'll double check against the other debug options in Section 12. Thank you, --nab -- 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/