On Sun, 27 Sep 2020 00:13:31 +0530 rohit maheshwari wrote: > >> > Also shouldn't we update this field or destroy the record before > >> the break on line 478? If more is set, and payload is lesser than the > >> max size, then we need to > >> hold on to get next sendpage and continue adding frags in the same > >> record. > >> So I don't think we need to do any update or destroy the record. Please > >> correct me if I am wrong here. > > > > Agreed, if more is set we should continue appending. > > > > What I'm saying is that we may exit the loop on line 478 or 525 without > > updating pending_open_record_frags. So if pending_open_record_frags is > > set, we'd be in a position where there is no data in the record, yet > > pending_open_record_frags is set. Won't subsequent cmsg send not cause > > a zero length record to be generated? > > Exit on line 478 can get triggered if sk_page_frag_refill() fails, and > > then by > Exit on line 478 can get triggered if sk_page_frag_refill() fails, > and then by exiting, it will hit line 529 and will return 'rc = > orig_size - size', so I am sure we don't need to do anything else > there.
What makes sure pending_open_record_frags is up to date on that exit path? > Exit on line 525 will be, due to do_tcp_sendpage(), and I > think pending_open_record_frags won't be set if this is the last > record. And if it is not the last record, do_tcp_sendpage will be > failing for a complete and correct record, that doesn't need to be > destroyed and at this very moment pending_open_record_frags > will suggest that there is more data (unrelated to current failing > record), which actually is correct. pending_open_record_frags does not mean more was set on previous call. It means there is an open record that needs to be closed in case cmsg needs to be sent. > I think, there won't be cmsg if pending_open_record_frags is set. cmsg comes from user space, what do you mean there won't be cmsg?
