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.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,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 37AD8CA9EB7 for ; Mon, 21 Oct 2019 09:54:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0E708218AC for ; Mon, 21 Oct 2019 09:54:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1571651697; bh=pwHFNv5WDHR0sm60/XRjrekzzcaxWPVm1eGnmHPT+nM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=KfDmywxg3203faeWnvFUBs+ALWd2KwKYzxONTkdyp6nOyK4JXlBm826EieFZdSBD8 np84aoOUMavq3U0TxsGDbNkQcf5R4KKYGJI2DNp2pKdGzHWK44Pd3duWMsB34gmWU6 EzEA8tRF6mxW5kdhfhNA4baDSyp3UfLjUz4nPdyo= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727919AbfJUJyw (ORCPT ); Mon, 21 Oct 2019 05:54:52 -0400 Received: from mail-lj1-f196.google.com ([209.85.208.196]:46009 "EHLO mail-lj1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727986AbfJUJyw (ORCPT ); Mon, 21 Oct 2019 05:54:52 -0400 Received: by mail-lj1-f196.google.com with SMTP id q64so12556617ljb.12; Mon, 21 Oct 2019 02:54:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=2ioAE/lCbaDrBdD3/67QZ6qtkIsHP6zYZBRbnhn/xwg=; b=j+p1sKTVDy2L5WN6rZftyUjjFaVETwIaG8orrxsgNdJoAv4FZmcK8HaCIyXoWICoXP YthOYHOMORvqYh4ErQLdcqU/mr5OC7K4SJLXOZlDLVVrTp5A/ienw7kQ0VgFgNIvMS2E Y8aqzv5raB/1QsuJ5PpW9fWd/6zEqrec4hkbbbGO5HlKh3QSRpPvBluk9RaCZOFjFrr6 x2lXEUNL3FCmXKQJnbLeOVaFQoAjnx1/aYzg19mAspGlvOSlxho/y9Fy01115ZdqutDr e4qEMQI9Xf/ktHr9GZzrrsXpaWH0xtv90wJUWyZE/Hh2HS5YvX02kqhMW1WDcLaveH59 vTGA== X-Gm-Message-State: APjAAAWL0D5wtlvFmWsRw/6CFVbaczsPxqr9OuZF59b6lrO8AvgoOW9+ rtDl2rtcTdJbTbcXdz1E2nc= X-Google-Smtp-Source: APXvYqwrLwkvwRh+ERVZri6+UvM2xfVCKPFTVDTPNLnM2AZOQEhfldYGPge2KvWGIkSk8YD190DGdQ== X-Received: by 2002:a2e:81da:: with SMTP id s26mr13975715ljg.192.1571651689431; Mon, 21 Oct 2019 02:54:49 -0700 (PDT) Received: from xi.terra (c-51f1e055.07-184-6d6c6d4.bbcust.telenor.se. [85.224.241.81]) by smtp.gmail.com with ESMTPSA id c69sm11510092ljf.32.2019.10.21.02.54.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 21 Oct 2019 02:54:48 -0700 (PDT) Received: from johan by xi.terra with local (Exim 4.92.2) (envelope-from ) id 1iMUOy-00005x-If; Mon, 21 Oct 2019 11:55:01 +0200 Date: Mon, 21 Oct 2019 11:55:00 +0200 From: Johan Hovold To: Daniel Vetter Cc: Johan Hovold , Rob Clark , Sean Paul , Fabien Dessenne , Mauro Carvalho Chehab , Harald Freudenberger , David Airlie , Heiko Carstens , Vasily Gorbik , Christian Borntraeger , linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, linux-s390@vger.kernel.org, Greg Kroah-Hartman , Al Viro Subject: Re: [PATCH 0/4] treewide: fix interrupted release Message-ID: <20191021095500.GE24768@localhost> References: <20191010131333.23635-1-johan@kernel.org> <20191010135043.GA16989@phenom.ffwll.local> <20191011093633.GD27819@localhost> <20191014084847.GD11828@phenom.ffwll.local> <20191014161326.GO13531@localhost> <20191015140726.GN11828@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191015140726.GN11828@phenom.ffwll.local> User-Agent: Mutt/1.12.2 (2019-09-21) Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On Tue, Oct 15, 2019 at 04:07:26PM +0200, Daniel Vetter wrote: > On Mon, Oct 14, 2019 at 06:13:26PM +0200, Johan Hovold wrote: > > On Mon, Oct 14, 2019 at 10:48:47AM +0200, Daniel Vetter wrote: > > > Do you have a legit usecase for interruptible sleeps in fops->release? > > > > The tty layer depends on this for example when waiting for buffered > > writes to complete (something which may never happen when using flow > > control). > > > > > I'm not even sure killable is legit in there, since it's an fd, not a > > > process context ... > > > > It will be run in process context in many cases, and for ttys we're good > > AFAICT. > > Huh, read it a bit, all the ->shutdown callbacks have void return type. > But there's indeed interruptible sleeps in there. Doesn't this break > userspace that expects that a close() actually flushes the tty? This behaviour has been there since "forever" so the problem is rather the other way round; changing it now might break user space. > Imo if you're ->release callbacks feels like it should do a wait to > guaranteed something userspace expects, then doing a > wait_interruptible/killable feels like a bug. Or alternatively, the wait > isn't really needed in the first place. Posix says that the final tty close should cause any output to be sent. And as mentioned before, due to flow control this may never finish. So for usability reasons, you want to be able to interrupt that final close, while removing the flush completely would break applications currently expecting output to be flushed. Also note that we have an interface for controlling how long to wait for data to be sent (typically 30 s by default, but can be set to wait forever). > > > > The return value from release() is ignored by vfs, and adding a splat in > > > > __fput() to catch these buggy drivers might be overkill. > > > > > > Ime once you have a handful of instances of a broken pattern, creating a > > > check for it (under a debug option only ofc) is very much justified. > > > Otherwise they just come back to life like the undead, all the time. And > > > there's a _lot_ of fops->release callbacks in the kernel. > > > > Yeah, you have a point. > > > > But take tty again as an example, the close tty operation called from > > release() is declared void so there's no propagated return value for vfs > > to check. > > > > It may even be better to fix up the 100 or so callbacks potentially > > returning non-zero and make fops->release void so that the compiler > > would help us catch any future bugs and also serve as a hint for > > developers that returning errnos from fops->release is probably not > > what you want to do. > > > > But that's a lot of churn of course. > > Hm indeed ->release has int as return type. I guess that's needed for > file I/O errno and similar stuff ... > > Still void return value doesn't catch funny stuff like doing interruptible > waits and occasionally failing if you have a process that likes to use > signals and also uses some library somewhere to do something. In graphics > we have that, with Xorg loving signals for various things. Right, but since there arguable are legitimate uses for interruptible sleep at release(), I don't see how we can catch that at runtime. Johan From mboxrd@z Thu Jan 1 00:00:00 1970 From: Johan Hovold Subject: Re: [PATCH 0/4] treewide: fix interrupted release Date: Mon, 21 Oct 2019 11:55:00 +0200 Message-ID: <20191021095500.GE24768@localhost> References: <20191010131333.23635-1-johan@kernel.org> <20191010135043.GA16989@phenom.ffwll.local> <20191011093633.GD27819@localhost> <20191014084847.GD11828@phenom.ffwll.local> <20191014161326.GO13531@localhost> <20191015140726.GN11828@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20191015140726.GN11828-dv86pmgwkMBes7Z6vYuT8azUEOm+Xw19@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: freedreno-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "Freedreno" To: Daniel Vetter Cc: Al Viro , freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Vasily Gorbik , linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Airlie , linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Heiko Carstens , Johan Hovold , Fabien Dessenne , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Christian Borntraeger , Rob Clark , Harald Freudenberger , dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Greg Kroah-Hartman , Mauro Carvalho Chehab , Sean Paul , linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBPY3QgMTUsIDIwMTkgYXQgMDQ6MDc6MjZQTSArMDIwMCwgRGFuaWVsIFZldHRlciB3 cm90ZToKPiBPbiBNb24sIE9jdCAxNCwgMjAxOSBhdCAwNjoxMzoyNlBNICswMjAwLCBKb2hhbiBI b3ZvbGQgd3JvdGU6Cj4gPiBPbiBNb24sIE9jdCAxNCwgMjAxOSBhdCAxMDo0ODo0N0FNICswMjAw LCBEYW5pZWwgVmV0dGVyIHdyb3RlOgoKPiA+ID4gRG8geW91IGhhdmUgYSBsZWdpdCB1c2VjYXNl IGZvciBpbnRlcnJ1cHRpYmxlIHNsZWVwcyBpbiBmb3BzLT5yZWxlYXNlPwo+ID4gCj4gPiBUaGUg dHR5IGxheWVyIGRlcGVuZHMgb24gdGhpcyBmb3IgZXhhbXBsZSB3aGVuIHdhaXRpbmcgZm9yIGJ1 ZmZlcmVkCj4gPiB3cml0ZXMgdG8gY29tcGxldGUgKHNvbWV0aGluZyB3aGljaCBtYXkgbmV2ZXIg aGFwcGVuIHdoZW4gdXNpbmcgZmxvdwo+ID4gY29udHJvbCkuCj4gPiAKPiA+ID4gSSdtIG5vdCBl dmVuIHN1cmUga2lsbGFibGUgaXMgbGVnaXQgaW4gdGhlcmUsIHNpbmNlIGl0J3MgYW4gZmQsIG5v dCBhCj4gPiA+IHByb2Nlc3MgY29udGV4dCAuLi4KPiA+IAo+ID4gSXQgd2lsbCBiZSBydW4gaW4g cHJvY2VzcyBjb250ZXh0IGluIG1hbnkgY2FzZXMsIGFuZCBmb3IgdHR5cyB3ZSdyZSBnb29kCj4g PiBBRkFJQ1QuCj4gCj4gSHVoLCByZWFkIGl0IGEgYml0LCBhbGwgdGhlIC0+c2h1dGRvd24gY2Fs bGJhY2tzIGhhdmUgdm9pZCByZXR1cm4gdHlwZS4KPiBCdXQgdGhlcmUncyBpbmRlZWQgaW50ZXJy dXB0aWJsZSBzbGVlcHMgaW4gdGhlcmUuIERvZXNuJ3QgdGhpcyBicmVhawo+IHVzZXJzcGFjZSB0 aGF0IGV4cGVjdHMgdGhhdCBhIGNsb3NlKCkgYWN0dWFsbHkgZmx1c2hlcyB0aGUgdHR5PwoKVGhp cyBiZWhhdmlvdXIgaGFzIGJlZW4gdGhlcmUgc2luY2UgImZvcmV2ZXIiIHNvIHRoZSBwcm9ibGVt IGlzIHJhdGhlcgp0aGUgb3RoZXIgd2F5IHJvdW5kOyBjaGFuZ2luZyBpdCBub3cgbWlnaHQgYnJl YWsgdXNlciBzcGFjZS4KCj4gSW1vIGlmIHlvdSdyZSAtPnJlbGVhc2UgY2FsbGJhY2tzIGZlZWxz IGxpa2UgaXQgc2hvdWxkIGRvIGEgd2FpdCB0bwo+IGd1YXJhbnRlZWQgc29tZXRoaW5nIHVzZXJz cGFjZSBleHBlY3RzLCB0aGVuIGRvaW5nIGEKPiB3YWl0X2ludGVycnVwdGlibGUva2lsbGFibGUg ZmVlbHMgbGlrZSBhIGJ1Zy4gT3IgYWx0ZXJuYXRpdmVseSwgdGhlIHdhaXQKPiBpc24ndCByZWFs bHkgbmVlZGVkIGluIHRoZSBmaXJzdCBwbGFjZS4KClBvc2l4IHNheXMgdGhhdCB0aGUgZmluYWwg dHR5IGNsb3NlIHNob3VsZCBjYXVzZSBhbnkgb3V0cHV0IHRvIGJlIHNlbnQuCkFuZCBhcyBtZW50 aW9uZWQgYmVmb3JlLCBkdWUgdG8gZmxvdyBjb250cm9sIHRoaXMgbWF5IG5ldmVyIGZpbmlzaC4g U28KZm9yIHVzYWJpbGl0eSByZWFzb25zLCB5b3Ugd2FudCB0byBiZSBhYmxlIHRvIGludGVycnVw dCB0aGF0IGZpbmFsCmNsb3NlLCB3aGlsZSByZW1vdmluZyB0aGUgZmx1c2ggY29tcGxldGVseSB3 b3VsZCBicmVhayBhcHBsaWNhdGlvbnMKY3VycmVudGx5IGV4cGVjdGluZyBvdXRwdXQgdG8gYmUg Zmx1c2hlZC4KCkFsc28gbm90ZSB0aGF0IHdlIGhhdmUgYW4gaW50ZXJmYWNlIGZvciBjb250cm9s bGluZyBob3cgbG9uZyB0byB3YWl0IGZvcgpkYXRhIHRvIGJlIHNlbnQgKHR5cGljYWxseSAzMCBz IGJ5IGRlZmF1bHQsIGJ1dCBjYW4gYmUgc2V0IHRvIHdhaXQKZm9yZXZlcikuCgo+ID4gPiA+IFRo ZSByZXR1cm4gdmFsdWUgZnJvbSByZWxlYXNlKCkgaXMgaWdub3JlZCBieSB2ZnMsIGFuZCBhZGRp bmcgYSBzcGxhdCBpbgo+ID4gPiA+IF9fZnB1dCgpIHRvIGNhdGNoIHRoZXNlIGJ1Z2d5IGRyaXZl cnMgbWlnaHQgYmUgb3ZlcmtpbGwuCj4gPiA+IAo+ID4gPiBJbWUgb25jZSB5b3UgaGF2ZSBhIGhh bmRmdWwgb2YgaW5zdGFuY2VzIG9mIGEgYnJva2VuIHBhdHRlcm4sIGNyZWF0aW5nIGEKPiA+ID4g Y2hlY2sgZm9yIGl0ICh1bmRlciBhIGRlYnVnIG9wdGlvbiBvbmx5IG9mYykgaXMgdmVyeSBtdWNo IGp1c3RpZmllZC4KPiA+ID4gT3RoZXJ3aXNlIHRoZXkganVzdCBjb21lIGJhY2sgdG8gbGlmZSBs aWtlIHRoZSB1bmRlYWQsIGFsbCB0aGUgdGltZS4gQW5kCj4gPiA+IHRoZXJlJ3MgYSBfbG90XyBv ZiBmb3BzLT5yZWxlYXNlIGNhbGxiYWNrcyBpbiB0aGUga2VybmVsLgo+ID4gCj4gPiBZZWFoLCB5 b3UgaGF2ZSBhIHBvaW50Lgo+ID4gCj4gPiBCdXQgdGFrZSB0dHkgYWdhaW4gYXMgYW4gZXhhbXBs ZSwgdGhlIGNsb3NlIHR0eSBvcGVyYXRpb24gY2FsbGVkIGZyb20KPiA+IHJlbGVhc2UoKSBpcyBk ZWNsYXJlZCB2b2lkIHNvIHRoZXJlJ3Mgbm8gcHJvcGFnYXRlZCByZXR1cm4gdmFsdWUgZm9yIHZm cwo+ID4gdG8gY2hlY2suCj4gPiAKPiA+IEl0IG1heSBldmVuIGJlIGJldHRlciB0byBmaXggdXAg dGhlIDEwMCBvciBzbyBjYWxsYmFja3MgcG90ZW50aWFsbHkKPiA+IHJldHVybmluZyBub24temVy byBhbmQgbWFrZSBmb3BzLT5yZWxlYXNlIHZvaWQgc28gdGhhdCB0aGUgY29tcGlsZXIKPiA+IHdv dWxkIGhlbHAgdXMgY2F0Y2ggYW55IGZ1dHVyZSBidWdzIGFuZCBhbHNvIHNlcnZlIGFzIGEgaGlu dCBmb3IKPiA+IGRldmVsb3BlcnMgdGhhdCByZXR1cm5pbmcgZXJybm9zIGZyb20gZm9wcy0+cmVs ZWFzZSBpcyBwcm9iYWJseSBub3QKPiA+IHdoYXQgeW91IHdhbnQgdG8gZG8uCj4gPiAKPiA+IEJ1 dCB0aGF0J3MgYSBsb3Qgb2YgY2h1cm4gb2YgY291cnNlLgo+IAo+IEhtIGluZGVlZCAtPnJlbGVh c2UgaGFzIGludCBhcyByZXR1cm4gdHlwZS4gSSBndWVzcyB0aGF0J3MgbmVlZGVkIGZvcgo+IGZp bGUgSS9PIGVycm5vIGFuZCBzaW1pbGFyIHN0dWZmIC4uLgo+IAo+IFN0aWxsIHZvaWQgcmV0dXJu IHZhbHVlIGRvZXNuJ3QgY2F0Y2ggZnVubnkgc3R1ZmYgbGlrZSBkb2luZyBpbnRlcnJ1cHRpYmxl Cj4gd2FpdHMgYW5kIG9jY2FzaW9uYWxseSBmYWlsaW5nIGlmIHlvdSBoYXZlIGEgcHJvY2VzcyB0 aGF0IGxpa2VzIHRvIHVzZQo+IHNpZ25hbHMgYW5kIGFsc28gdXNlcyBzb21lIGxpYnJhcnkgc29t ZXdoZXJlIHRvIGRvIHNvbWV0aGluZy4gSW4gZ3JhcGhpY3MKPiB3ZSBoYXZlIHRoYXQsIHdpdGgg WG9yZyBsb3Zpbmcgc2lnbmFscyBmb3IgdmFyaW91cyB0aGluZ3MuCgpSaWdodCwgYnV0IHNpbmNl IHRoZXJlIGFyZ3VhYmxlIGFyZSBsZWdpdGltYXRlIHVzZXMgZm9yIGludGVycnVwdGlibGUKc2xl ZXAgYXQgcmVsZWFzZSgpLCBJIGRvbid0IHNlZSBob3cgd2UgY2FuIGNhdGNoIHRoYXQgYXQgcnVu dGltZS4KCkpvaGFuCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fCkZyZWVkcmVubyBtYWlsaW5nIGxpc3QKRnJlZWRyZW5vQGxpc3RzLmZyZWVkZXNrdG9wLm9y ZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ZyZWVkcmVu bw==