Re: [PATCH 0/4] w1: refcnt fix, skip non-error send, docs

David Fries
2014-02-04T05:51:50Z

On Tue, Feb 04, 2014 at 03:59:38AM +0400, zbr@ioremap.net wrote:
> Hi
>
> 03.02.2014, 05:15, "David Fries" <David@Fries.net>:
>
> >  I could submit these patches as in, which would require the previous
> >  set, or I could merge the documentation into the previous set and
> >  resubmit them all since they haven't made it into the kernel tree yet.
> >  Opinions?
> >
> >  Here's a small refcnt fix, skipping sending non-error messages, and
> >  documentation and comment updates.
> >
> >  non-error error messages:
> >  Currently every master or slave command is sending a response with
> >  w1_netlink_send_error no matter if there is an error or not.  This
> >  makes commands like list slaves W1_CMD_LIST_SLAVES or W1_CMD_READ
> >  return two messages, one with data and one without.  That is a problem
> >  with the list slaves because they are identical except for one having
> >  data and one not, and since there could be no slaves known to the
> >  kernel you can't just discard the no data case, unless the program
> >  were to expect two replies.  So I propose only sending the error reply
> >  if there is an error, in which case there wouldn't be a normal reply
> >  (such as read).  This would mean commands like write would no longer
> >  return a response unless there was an error.  If an application wanted
> >  to verify the kernel received the write message it could follow it by
> >  a read to verify the data or just that read came after write and had a
> >  response so write must have completed without error.  I think it is
> >  safe to do away with the extra replies.  If someone sees a big enough
> >  need for this, I could modify it so all commands return one response,
> >  with commands like write always calling send error even if there
> >  wasn't one.
>
> I created this protocol to handle cases like nothing is returned, but yet userspace knows
> operations has been completed. Also, you can not really change it at this time - there are
> already userspace application which may depend on the last ack to find out its request completed.
>
> Reference counter fix is correct, please submit it in the separate patch.

Help me understand what the protocol is supposed to be. Assuming
there aren't any errors, is there supposed to be a
w1_netlink_send_error generated reply per netlink packet (cn_msg), per
w1_netlink_msg, or per w1_netlink_cmd?

What about the cn_msg seq and ack values? I assume the kernel
response should carry the same seq number as the request, but what
should the ack be set to?

--