Hi Andy > -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Sent: Tuesday, June 22, 2021 10:37 PM > To: Justin He <Justin.He@xxxxxxx> > Cc: Petr Mladek <pmladek@xxxxxxxx>; Steven Rostedt <rostedt@xxxxxxxxxxx>; > Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>; Rasmus Villemoes > <linux@xxxxxxxxxxxxxxxxxx>; Jonathan Corbet <corbet@xxxxxxx>; Alexander > Viro <viro@xxxxxxxxxxxxxxxxxx>; Linus Torvalds <torvalds@linux- > foundation.org>; Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>; Eric > Biggers <ebiggers@xxxxxxxxxx>; Ahmed S. Darwish <a.darwish@xxxxxxxxxxxxx>; > linux-doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux- > fsdevel@xxxxxxxxxxxxxxx; Matthew Wilcox <willy@xxxxxxxxxxxxx>; Christoph > Hellwig <hch@xxxxxxxxxxxxx>; nd <nd@xxxxxxx> > Subject: Re: [PATCH v5 1/4] fs: introduce helper d_path_unsafe() > > On Tue, Jun 22, 2021 at 10:06:31PM +0800, Jia He wrote: > > This helper is similar to d_path() except that it doesn't take any > > seqlock/spinlock. It is typical for debugging purposes. Besides, > > an additional return value *prenpend_len* is used to get the full > > path length of the dentry, ingoring the tail '\0'. > > the full path length = end - buf - prepend_length - 1 > > Missed period at the end of sentence. > Okay > > Previously it will skip the prepend_name() loop at once in > > __prepen_path() when the buffer length is not enough or even negative. > > prepend_name_with_len() will get the full length of dentry name > > together with the parent recursively regardless of the buffer length. > > > If someone invokes snprintf() with small but positive space, > > prepend_name_with_len() moves and copies the string partially. > > > > More than that, kasprintf() will pass NULL _buf_ and _end_ as the > > parameters. Hence return at the very beginning with false in this case. > > These two paragraphs are talking about printf() interface, while patch has > nothing to do with it. Please, rephrase in a way that it doesn't refer to > the > particular callers. Better to mention them in the corresponding printf() > patch(es). > Okay > ... > > > * prepend_name - prepend a pathname in front of current buffer pointer > > - * @buffer: buffer pointer > > - * @buflen: allocated length of the buffer > > + * @p: prepend buffer which contains buffer pointer and allocated length > > > * @name: name string and length qstr structure > > Indentation issue btw, can be fixed in the same patch. Okay > > > * > > * With RCU path tracing, it may race with d_move(). Use READ_ONCE() to > > Shouldn't this be a separate change with corresponding Fixes tag? Sorry, I don't quite understand here. What do you want to fix? > > ... > > > +/** > > + * d_path_unsafe - return the full path of a dentry without taking > > + * any seqlock/spinlock. This helper is typical for debugging purposes. > > Seems you ignored my comment, or forget to test, or compile test with > kernel > doc validator enabled doesn't show any issues. If it's the latter, we have > to > fix kernel doc validator. > > TL;DR: describe parameters as well. > My bad. Apologize for that. > > + */ > > ... > > > + struct path root; > > + struct mount *mnt = real_mount(path->mnt); > > + DECLARE_BUFFER(b, buf, buflen); > > Can wee keep this in reversed xmas tree order? Sure 😊 -- Cheers, Justin (Jia He)
