On Tue 17-01-17 17:24:15, Chen, Tim C wrote: > > > + /* > > > + * Preemption need to be turned on here, because we may sleep > > > + * in refill_swap_slots_cache(). But it is safe, because > > > + * accesses to the per-CPU data structure are protected by a > > > + * mutex. > > > + */ > > > > the comment doesn't really explain why it is safe. THere are other users > > which are not using the lock. E.g. just look at free_swap_slot above. > > How can > > cache->slots_ret[cache->n_ret++] = entry; be safe wrt. > > pentry = &cache->slots[cache->cur++]; > > entry = *pentry; > > > > Both of them might touch the same slot, no? Btw. I would rather prefer this > > would be a follow up fix with the trace and the detailed explanation. > > > > The cache->slots_ret is protected by cache->free_lock and cache->slots is > protected by cache->free_lock.
Ohh, I have misread those names and considered them the same thing. Sorry about the confusion. I will look at code more deeply tomorrow. > They are two separate structures, one for > caching the slots returned and one for caching the slots allocated. So > they do no touch the same slots. We'll update the comments so it is clearer. That would be really appreciated. -- Michal Hocko SUSE Labs
