Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 17 Feb 2021 16:25:49 +0000
From:   Chris Down <chris@chrisdown.name>
To:     Petr Mladek <pmladek@suse.com>
Cc:     linux-kernel@vger.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
        John Ogness <john.ogness@linutronix.de>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Kees Cook <keescook@chromium.org>, kernel-team@fb.com
Subject: Re: code style: Re: [PATCH v4] printk: Userspace format enumeration
 support
Message-ID: <YC1DjeZmBWmJe35c@chrisdown.name>
References: <YCafCKg2bAlOw08H@chrisdown.name>
 <YCv9Xb7ePnDy9xRf@alley>
 <YCwAbGoVuZJspcx5@chrisdown.name>
 <YC0/xQTephcfo6GL@alley>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <YC0/xQTephcfo6GL@alley>
User-Agent: Mutt/2.0.5 (da5e3282) (2021-01-21)
Precedence: bulk

Petr Mladek writes:
>What about storing the pointer to struct pf_object into
>struct printk_fmt_sec *ps into the s->file->f_inode->i_private?
>Then we would not need any global list/table at all.

Unless I'm misreading the debugfs code, I think the following is possible:

open(f);
   debugfs_file_get(f);
   fops->open();
     inode->private = ps;
   debugfs_file_put(f);

remove_printk_fmt_sec(); /* kfree ps */

read(f);
   debugfs_file_get(f);
   fops->read();
     ps = inode->private;  /* invalid */
   debugfs_file_put(f);

That's the reason why the code looks up from the module address again during 
_read. Maybe I'm missing something? :-)

>> Oh, I meant to change the name for v4 but neglected to do so. Sounds good,
>> will do.
>
>Thanks a lot. I am sorry that I ask you to do so many changes.
>I talked about the style early enough to make the review easy.
>Also I think that it is not ideal and annoing to do these
>mass changes and refactoring when the code is already reviewed,
>tested, and functional.

Quite the opposite: thanks a lot for taking so much time to provide valuable 
feedback :-) As someone who mostly works on mm code, having you to provide 
feedback as printk maintainer is really helpful. Even if we disagree on some 
stuff, it's really important that we have a good shared understanding of what 
we eventually agree upon.