From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 584B9C433E2 for ; Fri, 29 May 2020 01:16:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3379C207F5 for ; Fri, 29 May 2020 01:16:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2438357AbgE2BQP (ORCPT ); Thu, 28 May 2020 21:16:15 -0400 Received: from fieldses.org ([173.255.197.46]:36924 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2438352AbgE2BQN (ORCPT ); Thu, 28 May 2020 21:16:13 -0400 Received: by fieldses.org (Postfix, from userid 2815) id 2C4A51510; Thu, 28 May 2020 21:16:12 -0400 (EDT) Date: Thu, 28 May 2020 21:16:12 -0400 From: "J. Bruce Fields" To: NeilBrown Cc: Jeff Layton , Linux FS-devel Mailing List Subject: Re: The file_lock_operatoins.lock API seems to be a BAD API. Message-ID: <20200529011612.GF20602@fieldses.org> References: <87a71s8u23.fsf@notabene.neil.brown.name> <20200528220112.GD20602@fieldses.org> <87y2pb7dvc.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87y2pb7dvc.fsf@notabene.neil.brown.name> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Fri, May 29, 2020 at 11:01:59AM +1000, NeilBrown wrote: > On Thu, May 28 2020, J. Bruce Fields wrote: > > > On Thu, May 28, 2020 at 04:14:44PM +1000, NeilBrown wrote: > >> I don't think we should just fix all those bugs in those filesystems. > >> I think that F_UNLCK should *always* remove the lock/lease. > >> I imaging this happening by *always* calling posix_lock_file() (or > >> similar) in the unlock case - after calling f_op->lock() first if that > >> is appropriate. > >> > >> What do people think? It there on obvious reason that is a non-starter? > > > > Isn't NFS unlock like close, in that it may be our only chance to return > > IO errors? > > Is it? fcntl() isn't documented as returning ENOSPC, EDQUOT or EIO. I'm probably wrong. Writes have to be acknowledged before we return from unlock, but that doesn't mean that's our only chance to return any errors we find at that point. > > But I guess you're not saying that unlock can't return errors, just that > > it should always remove the lock whether it returns 0 or not. > > No I wasn't, but I might. > One approach that I was considering for making the API more robust was > to propose a separate "unlock" file_operation. All unlock requests > would go through this, and it would have a 'void' return type. > Would that be sufficient to encourage programmers to handle their own > errors and not think they can punt? Maybe I regret bringing up errors.... As you say, the important thing is making sure the lock's cleaned up, and doing that in common code sounds like the way to guarantee it. > But yes - even if unlock returns an error, it should (locally) remove > any locks - much as 'close()' will always close the fd (if it was > actually open) even if it reports an error. That makes sense to me. --b.