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