lkml.org 
[lkml]   [2018]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH 1/2] mm: swap: check if swap backing device is congested or not
From
Date
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



\
 
 \ /
  Last update: 2018-12-19 20:00    [W:1.283 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site