On Sat, 18 Apr 2020 14:34:51 -0700 Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > On Sat, Apr 18, 2020 at 01:15:09PM -0700, Andrew Morton wrote: > > > --- a/ipc/util.c > > > +++ b/ipc/util.c > > > @@ -104,12 +104,20 @@ static const struct rhashtable_params ipc_kht_params = { > > > .automatic_shrinking = true, > > > }; > > > > > > +#ifdef CONFIG_CHECKPOINT_RESTORE > > > > The code grew a few additional CONFIG_CHECKPOINT_RESTORE ifdefs. > > What's going on here? Why is CRIU special in ipc/? > > "grew a few"? I added (this) one and deleted two others. From in the > middle of functions, like we usually prefer. > Oh. > > @@ -17,11 +17,11 @@ struct ipc_ids { > ... > #ifdef CONFIG_CHECKPOINT_RESTORE > - int next_id; > + int restore_id; > #endif > > > > +#define set_restore_id(ids, x) ids->restore_id = x > > > +#define get_restore_id(ids) ids->restore_id > > > +#else > > > +#define set_restore_id(ids, x) do { } while (0) > > > +#define get_restore_id(ids) (-1) > > > +#endif > > > > Well these are ugly. Can't all this be done in C? > > Would you rather see it done as: > > static inline void set_restore_id(struct ipc_ids *ids, int id) > { > #ifdef CONFIG_CHECKPOINT_RESTORE > ids->restore_id = id; > #endif > } > > static inline int get_restore_id(struct ipc_ids *ids) > { > #ifdef CONFIG_CHECKPOINT_RESTORE > return ids->restore_id; > #else > return -1; > #endif > } Looks nicer. Has type checking regardless of Kconfig. Doesn't have lval-and-rval in one case, neither in the other. Doesn't risk unused-var warnings dependent on Kconfig. Etc. Could also do #ifdef CONFIG_CHECKPOINT_RESTORE static inline void set_restore_id(struct ipc_ids *ids, int id) { ids->restore_id = id; } static inline int get_restore_id(struct ipc_ids *ids) { return ids->restore_id; return -1; } #else ...