Hi Darrick,
On 2024-05-22 23:56, Darrick Wong wrote:
> On Wed, May 22, 2024 at 04:46:58PM +0900, Sukrit Bhatnagar wrote:
>> When a swapfile is created for hibernation purposes, we always need
>> the starting physical block offset, which is usually determined using
>> userspace commands such as filefrag.
>
> If you always need this value, then shouldn't it be exported via sysfs
> or somewhere so that you can always get to it? The kernel ringbuffer
> can overwrite log messages, swapfiles can get disabled, etc.
I agree on using appropriate kernel interfaces instead of kernel log.
>> It would be good to have that value printed when we do swapon and get
>> that value directly from dmesg.
>>
>> Signed-off-by: Sukrit Bhatnagar <Sukrit.Bhatnagar@xxxxxxxx>
>> ---
>> mm/swapfile.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> index f6ca215fb92f..53c9187d5fbe 100644
>> --- a/mm/swapfile.c
>> +++ b/mm/swapfile.c
>> @@ -3264,8 +3264,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>> (swap_flags & SWAP_FLAG_PRIO_MASK) >> SWAP_FLAG_PRIO_SHIFT;
>> enable_swap_info(p, prio, swap_map, cluster_info);
>> - pr_info("Adding %uk swap on %s. Priority:%d extents:%d across:%lluk %s%s%s%s\n",
>> + pr_info("Adding %uk swap on %s. Priority:%d extents:%d start:%llu across:%lluk %s%s%s%s\n",
>> K(p->pages), name->name, p->prio, nr_extents,
>> + (unsigned long long)first_se(p)->start_block,
>
> Last time I looked, start_block was in units of PAGE_SIZE, despite
> add_swap_extent confusingly (ab)using the sector_t type. Wherever you
> end up reporting this value, it ought to be converted to something more
> common (like byte offset or 512b-block offset).
I could not find any swap-related entries in the sysfs, but there is
/proc/swaps which shows the enabled swaps in a table.
A column for this start offset could be added there, which as you have
mentioned, should be in a unit such as bytes instead of PAGE_SIZE
blocks.
> Also ... if this is a swap *file* then reporting the path and the
> physical storage device address is not that helpful. Exposing the block
> device major/minor and block device address would be much more useful,
> wouldn't it?
For exposing information about swap file path, I think it wouldn't make
much difference (at least for the hibernate case) as we can always do
the file-path -> bdev-path -> major:minor conversion in userspace.
> (Not that I have any idea what the "suspend process" in the cover letter
> refers to -- suspend and hibernate have been broken on xfs forever...)
By suspend process, I meant the series of steps taken when we trigger
hibernate's suspend-to-disk.
Not the task that started it. (Wrong choice of words, my bad).
--
Sukrit
