From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dmitry Monakhov Subject: Re: [PATCH 1/3] fs: fcntl add set_flags wrapper Date: Tue, 14 Oct 2014 19:30:47 +0400 Message-ID: <87bnpept20.fsf@openvz.org> References: <1412958576-32621-1-git-send-email-dmonakhov@openvz.org> <20141011133741.GB29004@infradead.org> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Cc: linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk To: Christoph Hellwig Return-path: Received: from mail-wg0-f51.google.com ([74.125.82.51]:62475 "EHLO mail-wg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932147AbaJNPbA (ORCPT ); Tue, 14 Oct 2014 11:31:00 -0400 Received: by mail-wg0-f51.google.com with SMTP id b13so10992752wgh.34 for ; Tue, 14 Oct 2014 08:30:58 -0700 (PDT) In-Reply-To: <20141011133741.GB29004@infradead.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --=-=-= Content-Type: text/plain Christoph Hellwig writes: >> + else { >> + spin_lock(&filp->f_lock); >> + filp->f_flags = (arg & SETFL_MASK) | >> + (filp->f_flags & ~SETFL_MASK); >> + spin_unlock(&filp->f_lock); > > Please move this into an exported generic_file_set_flags helper, that > the filesystems can use in their implementations instead of duplicating > it. Ok. Will do > > Also a more conceptual question: Basically any check the filesystems > may perform needs to be duplicated in open and ->set_flags. Any chance > to have the open path call into ->set_flags? After your point I've checked various ->open callbacks and found that most filesystem makes important decisions such as: cifs_open: if (file->f_flags & O_DIRECT && cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) { if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL) file->f_op = &cifs_file_direct_nobrl_ops; else file->f_op = &cifs_file_direct_ops; } } And I am completely agree that it is reasonable to move such functionality to ->set_flags and let ->open call ->set_flags internally. ->set_flags can determine that it is called from open by condition: (filp->f_flags & SETFL_MASK) == (arg & SETFL_MASK) I'll fix setfl to prevent ->set_flags if args are the same --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUPUGnAAoJEFzOBSYIXfveKX4P/j0SbjlAveacpIdKMdySe09Q qhJikiu0dlSl94o5iW0mbGsPstOTDbdtAcyXlE2z4n7hSrPTAnD4itmmA+8BqzrT LO947/xSjTpymqT070HluQU9xXG+uzN1Ll66hJPXrBb7x2FqC0Q51L5XwvNQifYW /zToCD/eHNPxKT6JSyHDa+e0x8+lWyxHUjMmAKiQY2bvx+7CL5XfC0pLB/VJHfic smzyNsXzUTBGEvYedg9yBOSKEyi7fOJLyYMyfvK0zT9JZ4Y8W/Ty6LthouiCtq7G MDCYucUkyVpz5sUQYMxyHysYspsz/Jg9yrrpz+cF/fpCTTh0wQHsxE7rwvjWNKhR TJ5O5VNMAOX+VUW9LymRjKD9Zl7hh8yOeN0iCYVo9l82CJzbMaz93532bWUbcIHj gP/AQV5MSdf76eHm+sp3Bmv4yh06+ffwcayAoyX1TBbvWaKfFnZ9ukPOszo7X2EG 85GJHkPNiw7M/MPeo0kqpOHpn05T3iQvU/KzMGSSW1h3O/KMbwsSqtYFkP+kabDz A5O9jnmYn87fwL6u4ntUbtgP+O03D7AMkcNIFh4TrO1d1phqC8hcbPd2H97T7LqG Y2AZSzBzxSBowSypVcsUBVjc0XbU8tnDTUWZANDnPVbRIDGoI2GVOJ18Xdyjyh7y 37U3Et+HWSCL+CFdT6iJ =TkXX -----END PGP SIGNATURE----- --=-=-=--