On Fri, Aug 30, 2013 at 03:30:14PM -0700, Linus Torvalds wrote:
> On Fri, Aug 30, 2013 at 2:44 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > Point... Actually, I wonder if _that_ could be a solution for ->d_name.name
> > printk races as well. Remember that story? You objected against taking
> > spinlocks in printk, no matter how specialized and how narrow the area
> > over which those are taken, but rcu_read_lock/rcu_read_unlock should be
> > OK... Something like %pd expecting dentry pointer and producing dentry
> > name. Sure, we still get garbage if we race with d_move(), but at least
> > it's a contained garbage that way...
>
> Yes, that sounds quite reasonable. For printk, we'd probably want to
> limit the max size and depth to something fairly small (32 bytes, max
> four deep or something), and we cannot take cwd/root into account
> since it can happen from interrupts, but other than that it doesn't
> sound horrible.
Hmm... OK, most of these suckers are actually doing just one component;
we can look into 'print the ancestors as well' later, but the minimal
variant would be something like this and it already covers a lot of those
guys. Comments?
diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 3e8cb73..259f8c3 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -168,6 +168,13 @@ UUID/GUID addresses:
Where no additional specifiers are used the default little endian
order with lower case hex characters will be printed.
+dentry names:
+ %pd
+
+ For printing dentry name; if we race with d_move(), the name might be
+ a mix of old and new ones, but it won't oops. %pd dentry is safer
+ equivalent of %s dentry->d_name.name we used to use.
+
struct va_format:
%pV
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 739a3636..941509e 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -26,6 +26,7 @@
#include <linux/math64.h>
#include <linux/uaccess.h>
#include <linux/ioport.h>
+#include <linux/dcache.h>
#include <net/addrconf.h>
#include <asm/page.h> /* for PAGE_SIZE */
@@ -532,6 +533,56 @@ char *string(char *buf, char *end, const char *s, struct printf_spec spec)
return buf;
}
+static void widen(char *buf, char *end, unsigned len, unsigned spaces)
+{
+ size_t size;
+ if (buf >= end) /* nowhere to put anything */
+ return;
+ size = end - buf;
+ if (size <= spaces) {
+ memset(buf, ' ', size);
+ return;
+ }
+ if (len) {
+ if (len > size - spaces)
+ len = size - spaces;
+ memmove(buf + spaces, buf, len);
+ }
+ memset(buf, ' ', spaces);
+}
+
+static noinline_for_stack
+char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_spec spec)
+{
+ int n;
+ const char *s;
+ char *p = buf;
+ char c;
+
+ rcu_read_lock();
+ s = ACCESS_ONCE(d->d_name.name);
+ for (n = 0; n != spec.precision && (c = *s++) != 0; n++) {
+ if (buf < end)
+ *buf = c;
+ buf++;
+ }
+ rcu_read_unlock();
+ if (n < spec.field_width) {
+ /* we want to pad the sucker */
+ unsigned spaces = spec.field_width - n;
+ if (!(spec.flags & LEFT)) {
+ widen(p, end, n, spaces);
+ return buf + spaces;
+ }
+ while (spaces--) {
+ if (buf < end)
+ *buf = ' ';
+ ++buf;
+ }
+ }
+ return buf;
+}
+
static noinline_for_stack
char *symbol_string(char *buf, char *end, void *ptr,
struct printf_spec spec, const char *fmt)
@@ -1253,6 +1304,8 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
spec.base = 16;
return number(buf, end,
(unsigned long long) *((phys_addr_t *)ptr), spec);
+ case 'd':
+ return dentry_name(buf, end, ptr, spec);
}
spec.flags |= SMALL;
if (spec.field_width == -1) {
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
