public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Fries <David@Fries.net>
To: linux-kernel@vger.kernel.org
Cc: Evgeniy Polyakov <zbr@ioremap.net>
Subject: [PATCH 0/4] w1: refcnt fix, skip non-error send, docs
Date: Sun,  2 Feb 2014 19:14:59 -0600	[thread overview]
Message-ID: <1391390104-31154-1-git-send-email-David@Fries.net> (raw)

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.

It seems the way I read Documentation/connector/connector.txt not all
requests (from user space), must have a reply, and it does say
delivery isn't guaranteed as memory pressure can discard them, so I
think it's safe to drop the no error sending.

refcnt:
hub 7-0:1.0: port 2 disabled by hub (EMI?), re-enabling...
which is where the ds2490 was attached, which it would normally
recover just fine, except this time the kernel just printed,
w1_master_driver w1_bus_master1: Waiting for w1_bus_master1 to become free: refcnt=2.
and I had to reboot.  It hasn't been reproduced by removing and
replugging the ds2490.  In digging through the code I did find a
refcnt leak that could cause the problem.  I verified it leaked a
refcnt by modifying my program to send a command without data, and
verified it is fixed after.  I wouldn't have thought my program would
have sent an incomplete request, so I don't know what actually caused
the original refcnt problem, so it may still be there.


             reply	other threads:[~2014-02-03  1:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-03  1:14 David Fries [this message]
2014-02-03  1:15 ` [PATCH 1/4] w1: fix netlink refcnt leak on error path David Fries
2014-02-03  1:15 ` [PATCH 2/4] w1: only send_error when there is an error David Fries
2014-02-03  1:15 ` [PATCH 3/4] w1: document struct w1_netlink_msg and struct w1_netlink_cmd David Fries
2014-02-03  1:15 ` [PATCH 4/4] w1: update cn_netlink_send documentation David Fries
2014-02-03 23:59 ` [PATCH 0/4] w1: refcnt fix, skip non-error send, docs zbr
2014-02-04  5:51   ` David Fries
2014-02-04 23:48     ` zbr
2014-02-07  5:58       ` David Fries
2014-02-07 21:23         ` zbr
2014-02-07 22:23           ` David Fries
2014-02-07 22:35             ` zbr

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1391390104-31154-1-git-send-email-David@Fries.net \
    --to=david@fries.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=zbr@ioremap.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox