Messages in this thread | | | Date | Tue, 17 Jan 2017 21:03:39 +0100 | From | Michal Hocko <> | Subject | Re: [Update][PATCH v5 7/9] mm/swap: Add cache for swap slots allocation |
| |
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
| |