On Thu, 2011-03-03 at 16:30 -0700, Colin McCabe wrote:
> Hi Jim,
>
> It's true that either one of these commits should do the trick, but I
> would rather keep a41865e323 to make it explicit that we don't want
> SIGPIPE. It's kind of a form of documentation in the code ("oh, I see
> they're not using SIGPIPE"), and more documentation is a good thing.
OK, that makes sense.
>
> The other issue is that we shouldn't alter the signal disposition of
> library users. So it's prudent to put MSG_NOSIGNAL on all of our calls
> to send() and sendmsg for consistency, in case we ever end up calling
> send() from a library user's in a thread that we didn't create. (I
> don't think we do currently, but we may in the future I guess.)
Right, I didn't think about the library case.
Thanks for filling me in.
-- Jim
>
> cheers,
> Colin
>
>
> On Thu, Mar 3, 2011 at 3:06 PM, Jim Schutt <[email protected]> wrote:
> > Hi Colin,
> >
> > On Thu, 2011-03-03 at 14:53 -0700, Colin McCabe wrote:
> >> Oh, SIGPIPE, my old nemesis. I should have guessed!
> >>
> >> I think it's time to block SIGPIPE everywhere... It's much better to
> >> get EPIPE than to use a signal handler for this.
> >
> > I saw your commit d1fce13f9855.
> >
> > It seems like a41865e323 can be reverted, and
> > everything should work correctly?
> >
> > -- Jim
> >
> >>
> >> regards,
> >> Colin
> >>
> >>
> >> On Thu, Mar 3, 2011 at 12:55 PM, Yehuda Sadeh Weinraub
> >> <[email protected]> wrote:
> >> > On Thu, Mar 3, 2011 at 12:47 PM, Jim Schutt <[email protected]> wrote:
> >> >> Has something maybe changed in signal handling recently?
> >> >>
> >> >> Maybe SIGPIPE used to be blocked, and sendmsg() would
> >> >> return -EPIPE, but now it's not blocked and not handled?
> >> >>
> >> >> This bit in linux-2.6.git/net/core/stream.c is what made
> >> >> me wonder, but maybe it's a red herring:
> >> >>
> >> >> int sk_stream_error(struct sock *sk, int flags, int err)
> >> >> {
> >> >> if (err == -EPIPE)
> >> >> err = sock_error(sk) ? : -EPIPE;
> >> >> if (err == -EPIPE && !(flags & MSG_NOSIGNAL))
> >> >> send_sig(SIGPIPE, current, 0);
> >> >> return err;
> >> >> }
> >> >
> >> > It was actually just changed at
> >> > 35c4a9ffeadfe202b247c8e23719518a874f54e6, so if you're on latest
> >> > master then it might be it. You can try reverting that commit, or can
> >> > try this:
> >> >
> >> > index da22c7c..6f746d4 100644
> >> > --- a/src/msg/SimpleMessenger.cc
> >> > +++ b/src/msg/SimpleMessenger.cc
> >> > @@ -1991,7 +1991,7 @@ int SimpleMessenger::Pipe::do_sendmsg(int sd,
> >> > struct msghdr *msg, int len, bool
> >> > assert(l == len);
> >> > }
> >> >
> >> > - int r = ::sendmsg(sd, msg, more ? MSG_MORE : 0);
> >> > + int r = ::sendmsg(sd, msg, MSG_NOSIGNAL | (more ? MSG_MORE : 0));
> >> > if (r == 0)
> >> > dout(10) << "do_sendmsg hmm do_sendmsg got r==0!" << dendl;
> >> > if (r < 0) {
> >> >
> >> >
> >> > Yehuda
> >> >>
> >> >> -- Jim
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> >> >> the body of a message to [email protected]
> >> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> >>
> >> >
> >>
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
