Messages in this thread | | | Subject | Re: [RFC PATCH 1/2] mm: swap: check if swap backing device is congested or not | From | Tim Chen <> | Date | Wed, 19 Dec 2018 11:00:27 -0800 |
| |
On 12/19/18 10:40 AM, Yang Shi wrote: > >
>>>> I don't think your dereference inode = si->swap_file->f_mapping->host >>>> is always safe. You should do it only when (si->flags & SWP_FS) is true. >>> Do you mean it is not safe for swap partition? >> The f_mapping may not be instantiated. It is only done for SWP_FS. > > Really? I saw the below calls in swapon: > > swap_file = file_open_name(name, O_RDWR|O_LARGEFILE, 0); > ... > p->swap_file = swap_file; > mapping = swap_file->f_mapping; > inode = mapping->host; > ... > > Then the below code manipulates the inode. > > And, trace shows file_open_name() does call blkdev_open if it is turning block device swap on. And, blkdev_open() would return instantiated address_space and inode. > > Am I missing something? >
I was trying to limit the congestion logic for block devices backed swap. So the check I had in mind should really be "si->flags & SWP_BLKDEV" instead of si->flags & SWP_FS. I was concerned that there could be other use cases where the inode dereference is invalid.
Looking at the code a bit more, looks like swap_cluster_readahead is not used for other special case swap usage (like page migration). So you would a proper swapfile and inode here. But I think it is still a good idea to have a check for SWP_BLKDEV in si->flags.
Thanks.
Tim
| |