Messages in this thread | | | Date | Tue, 26 Sep 2017 00:12:46 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH 2/5] x86/intel_rdt: Add diagnostics when writing the schemata file |
| |
On Mon, 25 Sep 2017, Luck, Tony wrote: > On Mon, Sep 25, 2017 at 04:04:07PM +0200, Thomas Gleixner wrote: > > On Mon, 18 Sep 2017, Luck, Tony wrote: > > > @@ -208,14 +241,19 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, > > > char *tok, *resname; > > > int closid, ret = 0; > > > > > > + seq_buf_clear(&last_cmd_status); > > > + > > > /* Valid input requires a trailing newline */ > > > - if (nbytes == 0 || buf[nbytes - 1] != '\n') > > > + if (nbytes == 0 || buf[nbytes - 1] != '\n') { > > > + seq_buf_puts(&last_cmd_status, "no trailing newline\n"); > > > return -EINVAL; > > > + } > > > buf[nbytes - 1] = '\0'; > > > > In all other instances you access last_cmd_status within the > > rdtgroup_kn_lock_live() protected section, which also serializes the show() > > function via rdtgroup_mutex. Here you do it outside for obvious reasons, > > but that opens a can of evil worms ... > > Indeed. > > > Can you please provide and use two helpers - last_cmd_buf_clear() and > > last_cmd_buf_puts() - which both have a > > lockdep_assert_held(&rdtgroup_mutex) inside to make sure that we don't end > > up with unprotected access accidentally? > > Sure. In progress. But I also need a last_cmd_printf(), which for some > reason is giving me grief. In the header file I put: > > +static inline void last_cmd_printf(const char *fmt, ...) > +{ > + va_list ap; > + > + va_start(ap, fmt); > + lockdep_assert_held(&rdtgroup_mutex); > + seq_buf_printf(&last_cmd_status, fmt, ap);
seq_buf_vprintf() is your friend. It takes va_list as last argument.
While at it can you please make it a proper function? No point for inlining that.
Thanks,
tglx
| |