Re: [PATCH v2 1/1] eventfd: implementation of EFD_MASK flag

From: Martin Sustrik
Date: Thu Feb 14 2013 - 22:42:43 EST


Hi Andrew,

Thanks for the detailed code review! I'll have a look at all the problems you've pointed out, however, one quick question:

- ret = seq_printf(m, "eventfd-count: %16llx\n",
- (unsigned long long)ctx->count);
+ if (ctx->flags& EFD_MASK) {
+ ret = seq_printf(m, "eventfd-mask: %x\n",
+ (unsigned)ctx->mask.events);
+ } else {
+ ret = seq_printf(m, "eventfd-count: %16llx\n",
+ (unsigned long long)ctx->count);
+ }
spin_unlock_irq(&ctx->wqh.lock);

This is a non-back-compatible userspace interface change. A procfs
file which previously displayed

eventfd-count: nnnn

can now also display

eventfd-mask: nnnn

So existing userspace could misbehave.

Please fully describe the proposed interface change in the changelog.
That description should include the full pathname of the procfs file
and example before-and-after output and a discussion of whether and why
the risk to existing userspace is acceptable.

I am not sure what the policy is here. Is not printing out the state of the object acceptable way to maintain backward compatibility? If not so, does new type of object require new procfs file, which, AFAIU, is the only way to retain full backward compatibility?

Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/