Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Message-ID: <58037a2f-cbae-4bc1-bea4-604f6951b709@oracle.com>
Date:   Wed, 13 Dec 2023 13:03:30 +0000
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 04/16] fs: Increase fmode_t size
To:     Jan Kara <jack@suse.cz>
Cc:     axboe@kernel.dk, kbusch@kernel.org, hch@lst.de, sagi@grimberg.me,
        jejb@linux.ibm.com, martin.petersen@oracle.com, djwong@kernel.org,
        viro@zeniv.linux.org.uk, brauner@kernel.org, dchinner@redhat.com,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-nvme@lists.infradead.org, linux-xfs@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, tytso@mit.edu, jbongio@google.com,
        linux-scsi@vger.kernel.org, ming.lei@redhat.com,
        jaswin@linux.ibm.com, bvanassche@acm.org
References: <20231212110844.19698-1-john.g.garry@oracle.com>
 <20231212110844.19698-5-john.g.garry@oracle.com>
 <20231213112026.kkfcwtg64kiadhn5@quack3>
Content-Language: en-US
From:   John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20231213112026.kkfcwtg64kiadhn5@quack3>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TGMwaHlUMkI3eHdVNEhCWUVWR1FxZVQxN1NGS05GZnFoOWJSaXFMR3VzVkdp?=
 =?utf-8?B?cEhSdWQvYStrbE9wbTQ5dmd5eU1wUXJ3WXkxVVc0NmNTOHlDV28vblhhK3RX?=
 =?utf-8?B?SThBb3NRcVE2TUxlaUNnS1djNVFvc0dKazduRnBORlRURmRUdERJOVN1UGZU?=
 =?utf-8?B?OC9udVBlOTZuTTVudTBiOVJaaTR0OFNIektJcllJc0V6bFhsTTVvTGZZa256?=
 =?utf-8?B?elhIQ2VkNVM2bEdacnd6UHFDUEV6bnpxdTliUHBQRXJYNVlFcWpId3cvODhQ?=
 =?utf-8?B?TEhINnRrZXB0OEVtaTYxUUZsaDcyYytJOE1HN1ZEUkdGYWVYWGtCbkRXbnVp?=
 =?utf-8?B?SHkwblpVNEdUQmVadm5HVFJLRGlielgvOFBZUEpkQStVa2lYQXVKQXFVSnBG?=
 =?utf-8?B?Qy9OUlBCcVBTREJvU3EvZ0tIYm9lMllkYUsxZDdOdkJESnFxWWdIRWFaNEFv?=
 =?utf-8?B?VFEwZmdzaE1HNnU0L3pJWEthZTFDdS9GTVVmY2doalkxVmkwbFZ4VnJEUFhS?=
 =?utf-8?B?TFJIVkorRkY1RXFLUkZVZ0RSMVZ1ZW9Dakl5NVhNdnhjb0Y3aktGdzdkeFAz?=
 =?utf-8?B?WnB1M1crUGxrdjlLeHZpaWptRFRiQlRoZ1QrM2J2Q3B6UVRFMlFEMEo4RVZj?=
 =?utf-8?B?TE5HYmErM3IrTk5KL2ZCK1pGdERCZWVaQVdmQjRyZlN4b1Z4RGVDWENuZkpp?=
 =?utf-8?B?d1ZSTXlONkhveVdKdWFIQklIUFJhS2FUMURYTzZkQmYrQTAzRnNjWWZaR210?=
 =?utf-8?B?MkNZYjdDc3ZZT3FSREZlNE9aR20zd2JhSE1EdWxyTVBkWWtxNFZhTTZRS0xO?=
 =?utf-8?B?Q0VPYVJ2cWZkUlp2eThPdVE3cit6RERvczhtUzZ5RU5Bc2hISno4S3poK2pQ?=
 =?utf-8?B?bGFkUlhuV2l6Y3B0VUNBSjkycmVzVzFGTituVnhHcVg1T0hoMWFXeXd1cE5M?=
 =?utf-8?B?cjB5VkduV2RUSHlYdkIraHl4elRycWMxcU9oRXN4ejJheGhpaHVxYzNwenVE?=
 =?utf-8?B?dHVsaWlrSEY2eE1nSDJ3WTR6bmowMG01dS81U3c3clFFZzQ0bXc2ZDg2MElU?=
 =?utf-8?B?bUd1SFRGeFRVY1BSM0s2VXQ3V0p0U2VLZWxNSktIdHFoakFRZFZndHpHNnd4?=
 =?utf-8?B?UmJXKzI0SUFneEloakNxVnNGM3BucjBGbGRCV1F6MWFiVUpSYXhUQ0JESmRv?=
 =?utf-8?B?djltbndsUG9YTFRrZWI5cFNMR21HVDBoMXUrWXI2c3pWOG1RMUM4ZzZXa3RS?=
 =?utf-8?B?QmpHL0p5Ry9DdTBjQzhJSFEzenhKZE5iZUMxRERRNG44RVkvYkI4SGVpQmhu?=
 =?utf-8?B?YWE1V0cyMUtRbU1yNjlXc0EySUhhVzdDNXNFYndwVzhPZ3BENmVkSGpoWTRT?=
 =?utf-8?B?NG1Qa2thZGJZYW91R1l6MjhWbUFiLzVhWEVFaSsyY2pRSDlIa2dMdFFFb2lT?=
 =?utf-8?B?b3lObzE5Qlllc1pHdWY1SkFrblRqcHo5ZEowRFBwWUJnNTMrcVVGbHNEOFRm?=
 =?utf-8?B?WWJXNElNd1RJRVAxQkVTdFNXNC9RV1gza3JiUnlRTWdFSUtJUUppR3h6QWpz?=
 =?utf-8?B?NEpOck41emltNjFuZCt4WXhSaVJESWlNWFdKQWdYNVVsNE9wd1g5b3Jac214?=
 =?utf-8?B?RFdzdDA1NTA0eGJwdDlZcUpidS9yQXhZVS94Y3g2cmZFMXVMZjNRSVhvRHNH?=
 =?utf-8?B?REN6eS9Ba3hUQXc2blBaaVk5YlhiOUw2VVYwZ0tkcmR6dE1nZUdmYVJLWGQ1?=
 =?utf-8?B?S1lWQUI2NHpvUndtVlNVL0tWNjdQVEVIbjJ4M016dEJueEhNNWdWb0dHL05D?=
 =?utf-8?B?akhwLzlCMk9NV2o0TWFJK2N5RmpNSVdoN2xwaU5NUVJ6R0grRnNpMzdBRkhn?=
 =?utf-8?B?ekttd3hPRHNaTy95SVpPM2kxdTFHbktrdDcyYVF2aHNnN2JrVkFJcFhnWEw2?=
 =?utf-8?B?RHlKZUxNTXloTjdkaVJBL3NFcUZXSWRKZXBiNFZGeUVPeEVwTXdQbUVGRUdS?=
 =?utf-8?B?eHRHcXgrL01NUmpEWXVBbnV5MTJsSVBpSEhKWFRoVnJRQmJHWmJhenRZUjY5?=
 =?utf-8?B?WnVzQmRrRzhDa3FNVjJtL3VSNldLaE83UHdBSmVpS2xPaHhvbXNyLzJIcVll?=
 =?utf-8?Q?4TNFRFKy0lChy0vQ3k+66rw8O?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: R/bBEITkTNorZltqYQB3A3B/EkL7XOK7x0HkzoywdaGSAVzk4hK0gaWEx0P8ROzCFolSGEsXseCbb27eh4Xx20N5HuanbX5U9AOkD7pBVEOd87Sf9avIXNcyN0UoGBex2WRdhsznU9fmMuS1sCZCYIUVQXuroxDszb4OA7DcSWwdf3pAm8A4kaJTZoeurhrPBx1RfNTsIsLlGeqceFL5g5oXolFFnzsssiOxU5Tu6irjTm1lADSRwttqRmMP5q+UUtYXCkTYJH3ZpS/OaAnioU8YKJTL/mWqOhZH4JQ51o+0AyXr6iUI4zLcwHrecDnnp/4atLZH7825SLvG/kg4VfegAQzMov3C5MzIG46+9UNATPaVBOJeTaEPv1FtGkM9d3wp4SDvL9nwSLWCdiCvv6RqiRG2A1MFyLRqxY/JaTxJYh+iTAUOTHdhuCYLs3c2o5BOBGF2MjJ3Hp1SuswSE9ltoNVsvQVB8uxXTZdf+02L+CLpVmFlRou+2VCxWFEfqXi4OjsZmgt7VjoQEsuQy9SKx+hKrmIBbf0bUw4wZLVX/a9n8AN9fAHql27dFCBDkSZQE+iIunUqJyZwWexLtClnAHhqi23uFAbODIngR1s=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 34eef5dd-fa49-4716-0076-08dbfbdbead0
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 13:03:36.1293
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: MPYxn79DxKk5iF4PIvNdtgDKvF2vxR6EbN16rBtgloKZZCLoAg3Z/xYoY1HnmJXjOSqSo0T/CjFT37Yq/Pi45A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR10MB6799
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-13_05,2023-12-13_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 suspectscore=0
 phishscore=0 malwarescore=0 spamscore=0 mlxlogscore=999 bulkscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2312130095
X-Proofpoint-ORIG-GUID: cE4HmDVkSSsEQnhapUKqYNZdoeYMRSYh
X-Proofpoint-GUID: cE4HmDVkSSsEQnhapUKqYNZdoeYMRSYh
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on howler.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Wed, 13 Dec 2023 05:04:25 -0800 (PST)

On 13/12/2023 11:20, Jan Kara wrote:
>> To allow for further expansion, increase from unsigned int to unsigned
>> long.
>>
>> Since the dma-buf driver prints the file->f_mode member, change the print
>> as necessary to deal with the larger size.
>>
>> Signed-off-by: John Garry<john.g.garry@oracle.com>
> Uh, Al has more experience with fmode_t changes so I'd defer final decision
> to him but to me this seems dangerous.

Ack

> Firstly, this breaks packing of
> struct file on 64-bit architectures and struct file is highly optimized for
> cache efficiency (see the comment before the struct definition).

 From pahole, I think that we still fit on the same 64B cacheline 
(x86_64), but some padding has been added.

Before:
struct file {
union {
	struct llist_node  f_llist; 	/*     0     8 */
                 struct callback_head f_rcuhead 
__attribute__((__aligned__(8))); /*     0    16 */
	unsigned int       f_iocb_flags;         /*     0     4 */
         } __attribute__((__aligned__(8)));	/*     0    16 */
	spinlock_t                 f_lock;	/*    16     4 */
	fmode_t                    f_mode;	/*    20     4 */
	atomic_long_t              f_count;	/*    24     8 */
	struct mutex               f_pos_lock;	/*    32    32 */
         /* --- cacheline 1 boundary (64 bytes) --- */

After:

struct file {
union {
	struct llist_node  f_llist	/*     0     8 */
                 struct callback_head f_rcuhead 
__attribute__((__aligned__(8))); /*     0    16 */
	unsigned int       f_iocb_flags;	/*     0     4 */
         } __attribute__((__aligned__(8)));	/*     0    16 */
	spinlock_t                 f_lock;	 /*    16     4 */

         /* XXX 4 bytes hole, try to pack */

         fmode_t                    f_mode;	/*    24     8 */
         atomic_long_t              f_count;	/*    32     8 */
         struct mutex               f_pos_lock;	/*    40    32 */
         /* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */

> Secondly
> this will probably generate warnings on 32-bit architectures as there
> sizeof(unsigned long) == sizeof(unsigned int) and so your new flags won't
> fit anyway?

Right, it would then need to be unsigned long long. Or add another 32b 
member for extended modes. There were no i386 build warnings.

Thanks,
John