On Wed, Dec 05 2018 at 1:04pm -0500, Jens Axboe <axboe@xxxxxxxxx> wrote: > On 12/5/18 11:03 AM, Mike Snitzer wrote: > > On Wed, Dec 05 2018 at 12:54pm -0500, > > Jens Axboe <axboe@xxxxxxxxx> wrote: > > > >> On 12/5/18 10:49 AM, Mike Snitzer wrote: > >>> On Wed, Dec 05 2018 at 12:30pm -0500, > >>> Jens Axboe <axboe@xxxxxxxxx> wrote: > >>> > >>>> There's also no need to pass in the cpu, if we're not running with > >>>> preempt disabled already we have a problem. > >>> > >>> Why should this be any different than the part_stat_* interfaces? > >>> __part_stat_add(), part_stat_read(), etc also use > >>> per_cpu_ptr((part)->dkstats, (cpu) accessors. > >> > >> Maybe audit which ones actually need it? To answer the specific question, > >> it's silly to pass in the cpu, if we're pinned already. That's true > >> both programatically, but also for someone reading the code. > > > > I understand you'd like to avoid excess interface baggage. But seems to > > me we'd be better off being consistent, when extending the percpu > > portion of block core stats, and then do an incremental to clean it all > > up. > > The incremental should be done first in that case, it'd be silly to > introduce something only to do a cleanup right after. OK, all existing code for these percpu stats should follow the pattern: int cpu = part_stat_lock(); <do percpu diskstats stuff> part_stat_unlock(); part_stat_lock() calls get_cpu() which does preempt_disable(). So to your point: yes we have preempt disabled. And yes we _could_ just use smp_processor_id() in callers rather than pass 'cpu' to them. Is that what you want to see? Mike -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
