linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Luis Chamberlain <mcgrof@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	Kees Cook <keescook@chromium.org>,
	Iurii Zaikin <yzaikin@google.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 15/23] seq_file: switch over direct seq_read method calls to seq_read_iter
Date: Fri, 17 Jul 2020 23:09:13 +0200	[thread overview]
Message-ID: <87eep9rgqu.fsf@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20200707174801.4162712-16-hch@lst.de>

Christoph Hellwig <hch@lst.de> writes:

> Switch over all instances used directly as methods using these sed
> expressions:
>
> sed -i -e 's/\.read\(\s*=\s*\)seq_read/\.read_iter\1seq_read_iter/g'

This sucks, really. I just got a patch against this converting the
changed version to DEFINE_SHOW_ATTRIBUTE(somefile) and thereby removing
the whole open coded gunk.

If we do a tree wide change like this, then can we pretty please use a
coccinelle script to convert all trivial instances to use
DEFINE_SHOW_ATTRIBUTE so we don't have to touch the same place over and
over.

Out of 375 places changed in your patch something about 2/3rd fall into
the trivial category:

static int debug_stats_open(struct inode *inode, struct file *filp)
{
	return single_open(filp, debug_stats_show, NULL);
}

static const struct file_operations debug_stats_fops = {
	.open		= debug_stats_open,
	.read		= seq_read,
	.llseek		= seq_lseek,
	.release	= single_release,
};

which can be replaced by:

DEFINE_SHOW_ATTRIBUTE(debug_stats);

removing 12 lines of gunk and one central place to do the iter change.

I'm pretty sure that quite some of the others which have only an
additional write function can be replaced by a new macro
DEFINE_RW_ATTRIBUTE() or such.

Needs some thought and maybe some cocci help from Julia, but that's way
better than this brute force sed thing which results in malformed crap
like this:

static const struct file_operations debug_stats_fops = {
	.open		= debug_stats_open,
	.read_iter		= seq_read_iter,
	.llseek		= seq_lseek,
	.release	= single_release,
};

and proliferates the copy and paste voodoo programming.

Thanks,

        tglx

  parent reply	other threads:[~2020-07-17 21:09 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-07 17:47 stop using ->read and ->write for kernel access v3 Christoph Hellwig
2020-07-07 17:47 ` [PATCH 01/23] cachefiles: switch to kernel_write Christoph Hellwig
2020-07-07 17:47 ` [PATCH 02/23] autofs: " Christoph Hellwig
2020-07-07 17:47 ` [PATCH 03/23] bpfilter: " Christoph Hellwig
2020-07-07 17:47 ` [PATCH 04/23] fs: unexport __kernel_write Christoph Hellwig
2020-07-07 17:47 ` [PATCH 05/23] fs: check FMODE_WRITE in __kernel_write Christoph Hellwig
2020-07-07 17:47 ` [PATCH 06/23] fs: implement kernel_write using __kernel_write Christoph Hellwig
2020-07-07 17:47 ` [PATCH 07/23] fs: remove __vfs_write Christoph Hellwig
2020-07-07 17:47 ` [PATCH 08/23] fs: don't change the address limit for ->write_iter in __kernel_write Christoph Hellwig
2020-07-29 20:50   ` Al Viro
2020-07-30  7:02     ` Christoph Hellwig
2020-07-07 17:47 ` [PATCH 09/23] fs: add a __kernel_read helper Christoph Hellwig
2020-07-07 17:47 ` [PATCH 10/23] integrity/ima: switch to using __kernel_read Christoph Hellwig
2020-07-07 17:47 ` [PATCH 11/23] fs: implement kernel_read " Christoph Hellwig
2020-07-07 17:47 ` [PATCH 12/23] fs: remove __vfs_read Christoph Hellwig
2020-07-07 17:47 ` [PATCH 13/23] fs: don't change the address limit for ->read_iter in __kernel_read Christoph Hellwig
2020-07-07 17:47 ` [PATCH 14/23] seq_file: add seq_read_iter Christoph Hellwig
2020-07-07 17:47 ` [PATCH 16/23] proc: remove a level of indentation in proc_get_inode Christoph Hellwig
2020-07-07 17:47 ` [PATCH 17/23] proc: cleanup the compat vs no compat file ops Christoph Hellwig
2020-07-07 17:47 ` [PATCH 18/23] proc: add a read_iter method to proc proc_ops Christoph Hellwig
2020-07-07 17:47 ` [PATCH 19/23] proc: switch over direct seq_read method calls to seq_read_iter Christoph Hellwig
2020-07-07 17:47 ` [PATCH 20/23] sysctl: Convert to iter interfaces Christoph Hellwig
2020-07-07 17:47 ` [PATCH 21/23] fs: don't allow kernel reads and writes without iter ops Christoph Hellwig
2020-07-07 17:48 ` [PATCH 22/23] fs: default to generic_file_splice_read for files having ->read_iter Christoph Hellwig
2020-07-30  0:05   ` Al Viro
2020-07-30  7:03     ` Christoph Hellwig
2020-07-30 15:08       ` Al Viro
2020-07-30 15:20         ` Christoph Hellwig
2020-07-30 16:17           ` Al Viro
2020-07-30 16:22             ` Al Viro
2020-07-30 16:31               ` Christoph Hellwig
2020-07-07 17:48 ` [PATCH 23/23] fs: don't allow splice read/write without explicit ops Christoph Hellwig
2020-07-07 20:24 ` stop using ->read and ->write for kernel access v3 Linus Torvalds
2020-07-08  6:07   ` Christoph Hellwig
2020-07-07 23:03 ` Stephen Rothwell
     [not found] ` <20200707174801.4162712-16-hch@lst.de>
2020-07-10 12:55   ` [PATCH 15/23] seq_file: switch over direct seq_read method calls to seq_read_iter Jon Hunter
2020-07-10 12:58     ` Christoph Hellwig
2020-07-11  6:48     ` Christoph Hellwig
2020-07-11 11:47       ` Jon Hunter
2020-07-17 21:09   ` Thomas Gleixner [this message]
2020-07-20  9:33     ` Christoph Hellwig
2020-07-29 20:59     ` Al Viro
2020-07-30  7:10       ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87eep9rgqu.fsf@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=hch@lst.de \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=sfr@canb.auug.org.au \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    --cc=yzaikin@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).