Received-SPF: pass (google.com: domain of linux-kernel+bounces-190459-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: "Sukrit.Bhatnagar@sony.com" <Sukrit.Bhatnagar@sony.com>
To: "Darrick J. Wong" <djwong@kernel.org>
CC: "Rafael J. Wysocki" <rafael@kernel.org>, Pavel Machek <pavel@ucw.cz>,
        Christian Brauner <brauner@kernel.org>,
        Andrew Morton
	<akpm@linux-foundation.org>,
        "linux-xfs@vger.kernel.org"
	<linux-xfs@vger.kernel.org>,
        "linux-pm@vger.kernel.org"
	<linux-pm@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org"
	<linux-fsdevel@vger.kernel.org>,
        "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: RE: [PATCH 2/2] mm: swap: print starting physical block offset in
 swapon
Thread-Topic: [PATCH 2/2] mm: swap: print starting physical block offset in
 swapon
Thread-Index: AQHarBuqKQMfUh7E1kGrHyDTMsOgnrGjWEaAgAeYMgA=
Date: Mon, 27 May 2024 11:02:04 +0000
Message-ID: 
 <TYAPR01MB40483619A52F712D9265D4EAF6F02@TYAPR01MB4048.jpnprd01.prod.outlook.com>
References: <20240522074658.2420468-1-Sukrit.Bhatnagar@sony.com>
 <20240522074658.2420468-3-Sukrit.Bhatnagar@sony.com>
 <20240522145637.GV25518@frogsfrogsfrogs>
In-Reply-To: <20240522145637.GV25518@frogsfrogsfrogs>
Accept-Language: en-US, ja-JP, en-GB
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?V0C9E/GHiYyFYeUZhuXYb7PmlrD00P++78Z0hmWRr8WaK76KqWmkHfgYxWuJ?=
 =?us-ascii?Q?jWv2L0Tn1+LyQZrFVo9YcPNwF+leXkqMVdUqHeYSFrZ1zkToHR7uxx2NmfvO?=
 =?us-ascii?Q?dOwAL7iyTT0inHcgbVjv/tTkldTJY/m6g1hjNr+EDO2RoGa09n73aF/OBwNX?=
 =?us-ascii?Q?tWJzs6QaN+VRXbSknVI/kI8J4+m4qyZYacovU5sneBAjNIEdCoum2TIYBb1W?=
 =?us-ascii?Q?5U6DRP3XLvRkpmbEoNNYDDmB1q56kkdmt9Nh3F+1Y7ftW5FnTSGp1Z6GnLE+?=
 =?us-ascii?Q?y6mpqYpikEO4IHnHQw1/V8xejBkYSiLrdQtOWQg9qEMEOmrUHRESMgYmxViy?=
 =?us-ascii?Q?cz1l7+CNGR2BQgZZ7IP28p4krscQWdSchESwAB1UYBz0qVdEt3TBxYUcZcKR?=
 =?us-ascii?Q?xjaQ/8SIlxdkaFjg3+rEyOpyixoZMJE008pKhFWjgUZLMkLoArrdwklcwjYQ?=
 =?us-ascii?Q?9n6uEaxa0pt3WrkZQxFAgP7TFQL7bVLLQxijJyFMFXrxK9BZ4MckB4ITQ+DP?=
 =?us-ascii?Q?gQZTQzrcmt6ctuze3yzEh/bOEBn5cjOqVCnDH3UFTLbvpJ7vPxbHKiAAIt80?=
 =?us-ascii?Q?NedBtYhCT2sD308UIYiCMArs4rULjaxll6+vreKE+wZWGaZZehaYqztQUihe?=
 =?us-ascii?Q?RdnIKwu53eKq8JZZNbwUX9hj/1/aCj8WY8nOh3OcllljJ65rzlTt4SE6gx3i?=
 =?us-ascii?Q?+g6No4ub/BMnZgBZnpKywYZ75a5IyFzgrgaYT7ZaTB1n3PZ5YMIHicqiK3uu?=
 =?us-ascii?Q?4YpPyiamyZkN1VXVQX3hE79RCyGuuidrpQPPmGs8QZiA9hVQfSgoih1npRgQ?=
 =?us-ascii?Q?UAz/Bh/AHh/e2B1KKlc60AdYdBh6T3EzIulVGdt6N+XufW9Dyl1y7Lu5h0bo?=
 =?us-ascii?Q?L4zplrJ3ux2oZ8dRwtoyhL8tl5f2yVmy//LkBMQuYHl3HUFrjwRrvKUq0Ubh?=
 =?us-ascii?Q?hKd1RaAU8cuEFHJHjWDOhRNl0t2W6Ka3JkWNUWmKBicQ4k3PSnb/faG9b5lY?=
 =?us-ascii?Q?gCMkgR5EYi3F3N1HXngEGnMQfCR27237tGx9FehxrgJhQycaYbr4pNGMEqP0?=
 =?us-ascii?Q?Dlx72mmPUrLC2MQC7Z2LmxjHkxB7LsTD6MD5dIxNK/CF6a1Qz4PjysEMczFu?=
 =?us-ascii?Q?am2pkLzvL7H/6ueQHDTeXwfEF/hTwZ6/ZrufFp7FPHtW8XKGACQOw8qyd8Td?=
 =?us-ascii?Q?bCx05ayjcfDh6WMAerIMpR6YuLOF+7Ume2xme6KFDP3YkMXaf6YLLMH3yafN?=
 =?us-ascii?Q?QYEKRJmF0UBCNMvUgnKHBWYY62fKG+NEWdgJtOi8aFbQP6BkdR3xuCeXOnuS?=
 =?us-ascii?Q?u26kXs/ijp/baTOJm4+mgcSfZdybneEMAR4wJd7FyVpcPitmCJ2N52wxPcY0?=
 =?us-ascii?Q?Ughh56hASzEuZmB/7rUmymzbd1TGzpZaxj2Ynz3wUQWrQhOKsnxBfew8m5UC?=
 =?us-ascii?Q?P0zn6A/A7YzcHjr5ggCjadvXEogZN8krML858fmwwqCkjSc8cL0EsLlnKRLt?=
 =?us-ascii?Q?tX0pcLqVOWh7m8g3yaVyyS50qnL4OI/xI0tjUFb+emg7tx9OMUfIOc+NaVi0?=
 =?us-ascii?Q?hFtkJ/FGQsrm6fC8trfwCRkLKVmrPQrfEf2LrkKC?=
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	zQQzIF6mWNO/1zvRLN1LgSU9+Zb9vCesZuA5z7wW4vWPv0f0j41LeVkYZN2MnV49TIgLUduPg1u1H9RETf60fKPHcCrkvodtDpS4wQ91WisJOF29qC7zOaLk72YT6KNRdjPWgcUCZ7dhCi+xElTz/aoKXjaYsyz+yn5CqSSY/SN2ZCBQIVTRgv2dXJO8lil7LCgGrBRonARHaNUocozuX2DeOW60UmqBpoFZYwrHQ1ps/XYodeDUQ0ut8YhLoEp/ySpCb8gVn+lUC4lbTacAtcq+IrsgcP28SYaeJMlYI7QvthXvcSoxFH9xsU7qfZZ3yYGqUNxJUEL3nr67vXA/qXS8NTA//jJIjKrVF342fNJ3aHuz2KVEZbZB+MaglgwG2OjWXcztX3DUYLhBymCVu9gJwhUTTGu2mb2SuAG8QnRkIs7BkE42c0k98fqvAlakYS/TFmcGZw2XWgf1oHJ8jtpGueoSq3n8ryJIFmJUVGCGJOOE+91zbAc7RfCE29xP4okhDyUvqmPhchqIkn9z6Yq/k0xTBNhD7gVeQQ8gYsSL3a9XqUeKtGWGp7kA6+J/xzNy+5arsUcvOfQX28iPlko6dODAx7iEicHGR5sNa9xL5M11bJdp3ZwjNTNUDyLc
X-OriginatorOrg: sony.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: TYAPR01MB4048.jpnprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6ebfa70b-e19a-427c-437a-08dc7e3c7199
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 May 2024 11:02:04.8251
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 66c65d8a-9158-4521-a2d8-664963db48e4
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 5F0ENkTMGyrKimV0xiYrJ+bj1lDiQ4KulsxRiE/j/d+4t1la8u0CBiRxj+gQwi77FHqHf+rjtXvmXbXGQ3bd0EfThkLRUvwVDeGrbE6hnf0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: OSZPR01MB6971
X-Proofpoint-GUID: qaKH6VcbNDH0wgtHq0YqaQzWSNs3bnDW
X-Proofpoint-ORIG-GUID: qaKH6VcbNDH0wgtHq0YqaQzWSNs3bnDW
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
X-Sony-Outbound-GUID: qaKH6VcbNDH0wgtHq0YqaQzWSNs3bnDW
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.12.28.16
 definitions=2024-05-27_01,2024-05-24_01,2024-05-17_01

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.
>=20
> 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.
>>=20
>> Signed-off-by: Sukrit Bhatnagar <Sukrit.Bhatnagar@sony.com>
>> ---
>>  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 *, speci=
alfile, 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 acro=
ss:%lluk %s%s%s%s\n",
>>  		K(p->pages), name->name, p->prio, nr_extents,
>> +		(unsigned long long)first_se(p)->start_block,
>=20
> 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.
=20
> (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