Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC 16/16] ext4: Add process name and pid in ext4_msg()
To:     "Theodore Y. Ts'o" <tytso@mit.edu>
Cc:     linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        Jan Kara <jack@suse.com>,
        "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>
References: <cover.1589086800.git.riteshh@linux.ibm.com>
 <3d99e1291b3bc8f2a78467d11b1a7a31393180fc.1589086800.git.riteshh@linux.ibm.com>
 <20200521182650.GC2946569@mit.edu>
From:   Ritesh Harjani <riteshh@linux.ibm.com>
Date:   Tue, 2 Jun 2020 10:30:24 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.5.0
MIME-Version: 1.0
In-Reply-To: <20200521182650.GC2946569@mit.edu>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Message-Id: <20200602050025.CA74CA407B@b06wcsmtp001.portsmouth.uk.ibm.com>
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

Hello Ted,

Sorry about the long delay. Had taken some time off and was tinkering 
around with another hobby project.

On 5/21/20 11:56 PM, Theodore Y. Ts'o wrote:
> On Sun, May 10, 2020 at 11:54:56AM +0530, Ritesh Harjani wrote:
>> This adds process name and pid for ext4_msg().
>> I found this to be useful. For e.g. below print gives more
>> info about process name and pid.
>>
>> [ 7671.131912]  [mount/12543] EXT4-fs (dm-0): mounted filesystem with ordered data mode. Opts: acl,user_xattr
> 
> I'm not entirely sure about adding the command/pid at the beginning of
> the message.  The way we do this in ext4_warning and ext4_err is to
> print that information like this:
> 
> 		printk(KERN_CRIT
> 		       "EXT4-fs error (device %s): %s:%d: comm %s: %pV\n",
> 		       sb->s_id, function, line, current->comm, &vaf);
> 
> ... and I wonder if it would make more sense to add something like to
> ext4_msg(), just out of consistency's sake.  Which of the debugging
> messages were you finding this to be most helpful?

Well earlier ext4_mb_show_ac() was using ext4_msg() function.
But I changed that to use mb_debug() msg in patch-14 of this series,
since mb_debug() is meant for those debug msgs.
So I am completely ok if we think this patch is unnecessary, that's also
why I kept this patch at the end of the series to check opinion of
others.

FWIW, the mballoc issue which I was seeing was mostly due to a multi-
threaded application. And without name/pid of the process/threads, it
was difficult to identify which debug msgs belonged to which threads.
For this reason I thought such info in ext4_msg() would also help in
future.


Thanks for taking both patch series!
-ritesh