Re: [PATCH v4 0/9] mm/swap: Regular page swap optimizations

From: Minchan Kim
Date: Tue Dec 27 2016 - 23:08:38 EST


On Wed, Dec 28, 2016 at 11:31:06AM +0800, Huang, Ying wrote:

< snip >

> >>> > Frankly speaking, although I'm huge user of bit_spin_lock(zram/zsmalloc
> >>> > have used it heavily), I don't like swap subsystem uses it.
> >>> > During zram development, it really hurts debugging due to losing lockdep.
> >>> > The reason zram have used it is by size concern of embedded world but server
> >>> > would be not critical so please consider trade-off of spinlock vs. bit_spin_lock.
> >>>
> >>> There will be one struct swap_cluster_info for every 1MB swap space.
> >>> So, for example, for 1TB swap space, the number of struct
> >>> swap_cluster_info will be one million. To reduce the RAM usage, we
> >>> choose to use bit_spin_lock, otherwise, spinlock is better. The code
> >>> will be used by embedded, PC and server, so the RAM usage is important.
> >>
> >> It seems you already increase swap_cluster_info 4 byte to support
> >> bit_spin_lock.
> >
> > The increment only occurs on 64bit platform. On 32bit platform, the
> > size is the same as before.
> >
> >> Compared to that, how much memory does spin_lock increase?
> >
> > The size of struct swap_cluster_info will increase from 4 bytes to 16
> > bytes on 64bit platform. I guess it will increase from 4 bytes to 8
> > bytes on 32bit platform at least, but I did not test that.
>
> Sorry, I make a mistake during test. The size of struct
> swap_cluster_info will increase from 4 bytes to 8 bytes on 64 bit
> platform. I think it will increase from 4 bytes to 8 bytes on 32 bit
> platform too (not tested).

Thanks for the information.
To me, it's not big when we consider spinlock's usefullness which helps
cache-line bouncing, lockdep and happy with RT people.
So, I vote spin_lock but I'm not in charge of deciding on that and your
opinion might be different still. If so, let's pass the decision to
maintainer.
Instead, please write down above content in description for maintainer to
judge it fairly.

Thanks.