From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934582AbcHaNHr (ORCPT ); Wed, 31 Aug 2016 09:07:47 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:37176 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934161AbcHaNHk (ORCPT ); Wed, 31 Aug 2016 09:07:40 -0400 Date: Wed, 31 Aug 2016 15:07:49 +0200 From: Greg Kroah-Hartman To: Nicolai Stange Cc: Brian Starkey , Liviu Dudau , LKML Subject: Re: [PATCH] debugfs: Add proxy function for the mmap file operation Message-ID: <20160831130749.GA23785@kroah.com> References: <20160729143459.2672-1-Liviu.Dudau@arm.com> <87oa5gpcu1.fsf@gmail.com> <87wpjzkrgn.fsf@gmail.com> <20160805101849.GA29995@e106950-lin.cambridge.arm.com> <8760rfzczy.fsf@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8760rfzczy.fsf@gmail.com> User-Agent: Mutt/1.7.0 (2016-08-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 05, 2016 at 01:11:45PM +0200, Nicolai Stange wrote: > Brian Starkey writes: > > > On Tue, Aug 02, 2016 at 07:31:36PM +0200, Nicolai Stange wrote: > >>Nicolai Stange writes: > >>> However, if you wish to have some mmapable debugfs file which *can* go > >>> away, introducing mmap support in the debugfs full proxy is perfectly > >>> valid. But please see below. > >> > >>Assuming that you've got such a use case, please consider resending your > >>patch along with the Cocci script below (and the Coccinelle team CC'ed, > >>of course). If OTOH your mmapable debugfs files are never removed, just > >>drop this message and use debugfs_create_file_unsafe() instead. > > > > So we do have an implementation using this, but it's likely we will > > keep it out-of-tree (it's a stop-gap until we can get a non-debugfs > > implementation of the functionality into mainline). > > > > Do you think it's worth merging this (and your cocci script) anyway to > > save someone else doing the same thing later? > > I personally think that having ->mmap() support in debugfs would be a > good thing to have in general and I expect there to be some further > demand in the future. Ugh, mmap in debugfs, that's funny. And sad... > But I also think that it is a little bit fragile in the current state: > how many people actually run the Cocci scripts on their changes? AFAICT, > even the kbuild test robot doesn't do this. And after all, the Cocci > script I provided could very well miss some obfuscated writes to > vma->vm_ops: if they aren't done from ->mmap() themselves, but from some > helper function invoked therein, for example. > > I would personally prefer a hand coded full_proxy_mmap() which WARN()s > if the proxied ->mmap() changes vma->vm_ops: > - this would add an extra safety net > - ->mmap() for debugfs files isn't performance critical > - and lastly, we're already doing something similar to this in > open_proxy_open(). Yes, that would be the best thing to do here. thanks, greg k-h