From: Chris Leech <hidden> Date: 2006-03-03 21:40:17
This patch series is the first full release of the Intel(R) I/O
Acceleration Technology (I/OAT) for Linux. It includes an in kernel API
for offloading memory copies to hardware, a driver for the I/OAT DMA memcpy
engine, and changes to the TCP stack to offload copies of received
networking data to application space.
These changes apply to DaveM's net-2.6.17 tree as of commit
2bd84a93d8bb7192ad8c23ef41008502be1cb603 ([IRDA]: TOIM3232 dongle support)
They are available to pull from
git://198.78.49.142/~cleech/linux-2.6 ioat-2.6.17
There are 8 patches in the series:
1) The memcpy offload APIs and class code
2) The Intel I/OAT DMA driver (ioatdma)
3) Core networking code to setup networking as a DMA memcpy client
4) Utility functions for sk_buff to iovec offloaded copy
5) Structure changes needed for TCP receive offload
6) Rename cleanup_rbuf to tcp_cleanup_rbuf
7) Add a sysctl to tune the minimum offloaded I/O size for TCP
8) The main TCP receive offload changes
--
Chris Leech [off-list ref]
I/O Acceleration Technology Software Development
LAN Access Division / Digital Enterprise Group
@@ -0,0 +1,361 @@+/*****************************************************************************+Copyright(c)2004-2006IntelCorporation.Allrightsreserved.++Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+underthetermsoftheGNUGeneralPublicLicenseaspublishedbytheFree+SoftwareFoundation;eitherversion2oftheLicense,or(atyouroption)+anylaterversion.++Thisprogramisdistributedinthehopethatitwillbeuseful,butWITHOUT+ANYWARRANTY;withouteventheimpliedwarrantyofMERCHANTABILITYor+FITNESSFORAPARTICULARPURPOSE.SeetheGNUGeneralPublicLicensefor+moredetails.++YoushouldhavereceivedacopyoftheGNUGeneralPublicLicensealongwith+thisprogram;ifnot,writetotheFreeSoftwareFoundation,Inc.,59+TemplePlace-Suite330,Boston,MA02111-1307,USA.++ThefullGNUGeneralPublicLicenseisincludedinthisdistributioninthe+filecalledLICENSE.+*****************************************************************************/+#include<linux/init.h>+#include<linux/module.h>+#include<linux/device.h>+#include<linux/dmaengine.h>+#include<linux/hardirq.h>+#include<linux/spinlock.h>+#include<linux/percpu.h>+#include<linux/rcupdate.h>++staticspinlock_tdma_list_lock;+staticLIST_HEAD(dma_device_list);+staticLIST_HEAD(dma_client_list);++/* --- sysfs implementation --- */++staticssize_tshow_memcpy_count(structclass_device*cd,char*buf)+{+structdma_chan*chan=container_of(cd,structdma_chan,class_dev);+unsignedlongcount=0;+inti;++for_each_cpu(i)+count+=per_cpu_ptr(chan->local,i)->memcpy_count;++sprintf(buf,"%lu\n",count);+returnstrlen(buf)+1;+}++staticssize_tshow_bytes_transferred(structclass_device*cd,char*buf)+{+structdma_chan*chan=container_of(cd,structdma_chan,class_dev);+unsignedlongcount=0;+inti;++for_each_cpu(i)+count+=per_cpu_ptr(chan->local,i)->bytes_transferred;++sprintf(buf,"%lu\n",count);+returnstrlen(buf)+1;+}++staticssize_tshow_in_use(structclass_device*cd,char*buf)+{+structdma_chan*chan=container_of(cd,structdma_chan,class_dev);++sprintf(buf,"%d\n",(chan->client?1:0));+returnstrlen(buf)+1;+}++staticstructclass_device_attributedma_class_attrs[]={+__ATTR(memcpy_count,S_IRUGO,show_memcpy_count,NULL),+__ATTR(bytes_transferred,S_IRUGO,show_bytes_transferred,NULL),+__ATTR(in_use,S_IRUGO,show_in_use,NULL),+__ATTR_NULL+};++staticvoiddma_async_device_cleanup(structkref*kref);++staticvoiddma_class_dev_release(structclass_device*cd)+{+structdma_chan*chan=container_of(cd,structdma_chan,class_dev);+kref_put(&chan->device->refcount,dma_async_device_cleanup);+}++staticstructclassdma_devclass={+.name="dma",+.class_dev_attrs=dma_class_attrs,+.release=dma_class_dev_release,+};++/* --- client and device registration --- */++/**+*dma_client_chan_alloc-trytoallocateachanneltoaclient+*@client:&dma_client+*+*Calledwithdma_list_lockheld.+*/+staticstructdma_chan*dma_client_chan_alloc(structdma_client*client)+{+structdma_device*device;+structdma_chan*chan;+unsignedlongflags;++/* Find a channel, any DMA engine will do */+list_for_each_entry(device,&dma_device_list,global_node){+list_for_each_entry(chan,&device->channels,device_node){+if(chan->client)+continue;++if(chan->device->device_alloc_chan_resources(chan)>=0){+kref_get(&device->refcount);+kref_init(&chan->refcount);+chan->slow_ref=0;+INIT_RCU_HEAD(&chan->rcu);+chan->client=client;+spin_lock_irqsave(&client->lock,flags);+list_add_tail_rcu(&chan->client_node,&client->channels);+spin_unlock_irqrestore(&client->lock,flags);+returnchan;+}+}+}++returnNULL;+}++/**+*dma_client_chan_free-releaseaDMAchannel+*@chan:&dma_chan+*/+voiddma_async_device_cleanup(structkref*kref);+voiddma_chan_cleanup(structkref*kref)+{+structdma_chan*chan=container_of(kref,structdma_chan,refcount);+chan->device->device_free_chan_resources(chan);+chan->client=NULL;+kref_put(&chan->device->refcount,dma_async_device_cleanup);+}++staticvoiddma_chan_free_rcu(structrcu_head*rcu){+structdma_chan*chan=container_of(rcu,structdma_chan,rcu);+intbias=0x7FFFFFFF;+inti;+for_each_cpu(i)+bias-=local_read(&per_cpu_ptr(chan->local,i)->refcount);+atomic_sub(bias,&chan->refcount.refcount);+kref_put(&chan->refcount,dma_chan_cleanup);+}++staticvoiddma_client_chan_free(structdma_chan*chan)+{+atomic_add(0x7FFFFFFF,&chan->refcount.refcount);+chan->slow_ref=1;+call_rcu(&chan->rcu,dma_chan_free_rcu);+}++/**+*dma_chans_rebalance-reallocatechannelstoclients+*+*WhenthenumberofDMAchannelinthesystemchanges,+*channelsneedtoberebalancedamongclients+*/+staticvoiddma_chans_rebalance(void)+{+structdma_client*client;+structdma_chan*chan;+unsignedlongflags;++spin_lock(&dma_list_lock);+list_for_each_entry(client,&dma_client_list,global_node){++while(client->chans_desired>client->chan_count){+chan=dma_client_chan_alloc(client);+if(!chan)+break;++client->chan_count++;+client->event_callback(client,chan,DMA_RESOURCE_ADDED);+}++while(client->chans_desired<client->chan_count){+spin_lock_irqsave(&client->lock,flags);+chan=list_entry(client->channels.next,structdma_chan,client_node);+list_del_rcu(&chan->client_node);+spin_unlock_irqrestore(&client->lock,flags);+client->chan_count--;+client->event_callback(client,chan,DMA_RESOURCE_REMOVED);+dma_client_chan_free(chan);+}+}+spin_unlock(&dma_list_lock);+}++/**+*dma_async_client_register-allocateandregistera&dma_client+*@event_callback:callbackfornotificationofchanneladdition/removal+*/+structdma_client*dma_async_client_register(dma_event_callbackevent_callback)+{+structdma_client*client;++client=kzalloc(sizeof(*client),GFP_KERNEL);+if(!client)+returnNULL;++INIT_LIST_HEAD(&client->channels);+spin_lock_init(&client->lock);++client->chans_desired=0;+client->chan_count=0;+client->event_callback=event_callback;++spin_lock(&dma_list_lock);+list_add_tail(&client->global_node,&dma_client_list);+spin_unlock(&dma_list_lock);++returnclient;+}++/**+*dma_async_client_unregister-unregisteraclientandfreethe&dma_client+*@client:+*+*ForcefreesanyallocatedDMAchannels,freesthe&dma_clientmemory+*/+voiddma_async_client_unregister(structdma_client*client)+{+structdma_chan*chan;++if(!client)+return;++rcu_read_lock();+list_for_each_entry_rcu(chan,&client->channels,client_node){+dma_client_chan_free(chan);+}+rcu_read_unlock();++spin_lock(&dma_list_lock);+list_del(&client->global_node);+spin_unlock(&dma_list_lock);++kfree(client);+dma_chans_rebalance();+}++/**+*dma_async_client_chan_request-requestDMAchannels+*@client:&dma_client+*@number:countofDMAchannelsrequested+*+*Clientscalldma_async_client_chan_request()tospecifyhowmany+*DMAchannelstheyneed,0tofreeallcurrentlyallocated.+*Theresultingallocations/freesareindicatedtotheclientviathe+*eventcallback.+*/+voiddma_async_client_chan_request(structdma_client*client,+unsignedintnumber)+{+client->chans_desired=number;+dma_chans_rebalance();+}++/**+*dma_async_device_register-+*@device:&dma_device+*/+intdma_async_device_register(structdma_device*device)+{+staticintid;+intchancnt=0;+structdma_chan*chan;++if(!device)+return-ENODEV;++init_completion(&device->done);+kref_init(&device->refcount);+device->dev_id=id++;++/* represent channels in sysfs. Probably want devs too */+list_for_each_entry(chan,&device->channels,device_node){+chan->local=alloc_percpu(typeof(*chan->local));+if(chan->local==NULL)+continue;++chan->chan_id=chancnt++;+chan->class_dev.class=&dma_devclass;+chan->class_dev.dev=NULL;+snprintf(chan->class_dev.class_id,BUS_ID_SIZE,"dma%dchan%d",+device->dev_id,chan->chan_id);++kref_get(&device->refcount);+class_device_register(&chan->class_dev);+}++spin_lock(&dma_list_lock);+list_add_tail(&device->global_node,&dma_device_list);+spin_unlock(&dma_list_lock);++dma_chans_rebalance();++return0;+}++/**+*dma_async_device_unregister-+*@device:&dma_device+*/+staticvoiddma_async_device_cleanup(structkref*kref){+structdma_device*device=container_of(kref,structdma_device,refcount);+complete(&device->done);+}++voiddma_async_device_unregister(structdma_device*device)+{+structdma_chan*chan;+unsignedlongflags;++spin_lock(&dma_list_lock);+list_del(&device->global_node);+spin_unlock(&dma_list_lock);++list_for_each_entry(chan,&device->channels,device_node){+if(chan->client){+spin_lock_irqsave(&chan->client->lock,flags);+list_del(&chan->client_node);+chan->client->chan_count--;+spin_unlock_irqrestore(&chan->client->lock,flags);+chan->client->event_callback(chan->client,chan,DMA_RESOURCE_REMOVED);+dma_client_chan_free(chan);+}+class_device_unregister(&chan->class_dev);+}++dma_chans_rebalance();++kref_put(&device->refcount,dma_async_device_cleanup);+wait_for_completion(&device->done);+}++staticint__initdma_bus_init(void)+{+spin_lock_init(&dma_list_lock);++returnclass_register(&dma_devclass);+}++subsys_initcall(dma_bus_init);++EXPORT_SYMBOL(dma_async_client_register);+EXPORT_SYMBOL(dma_async_client_unregister);+EXPORT_SYMBOL(dma_async_client_chan_request);+EXPORT_SYMBOL(dma_async_memcpy_buf_to_buf);+EXPORT_SYMBOL(dma_async_memcpy_buf_to_pg);+EXPORT_SYMBOL(dma_async_memcpy_pg_to_pg);+EXPORT_SYMBOL(dma_async_memcpy_complete);+EXPORT_SYMBOL(dma_async_memcpy_issue_pending);+EXPORT_SYMBOL(dma_async_device_register);+EXPORT_SYMBOL(dma_async_device_unregister);
From: Chris Leech <hidden> Date: 2006-03-03 21:40:46
Needed to be able to call tcp_cleanup_rbuf in tcp_input.c for I/OAT
Signed-off-by: Chris Leech <redacted>
---
include/net/tcp.h | 2 ++
net/ipv4/tcp.c | 10 +++++-----
2 files changed, 7 insertions(+), 5 deletions(-)
@@ -936,7 +936,7 @@ static int tcp_recv_urg(struct sock *sk,*calculationofwhetherornotwemustACKforthesakeof*awindowupdate.*/-staticvoidcleanup_rbuf(structsock*sk,intcopied)+voidtcp_cleanup_rbuf(structsock*sk,intcopied){structtcp_sock*tp=tcp_sk(sk);inttime_to_ack=0;
@@ -1085,7 +1085,7 @@ int tcp_read_sock(struct sock *sk, read_/* Clean up data we have read: This will do ACK frames. */if(copied)-cleanup_rbuf(sk,copied);+tcp_cleanup_rbuf(sk,copied);returncopied;}
@@ -1219,7 +1219,7 @@ int tcp_recvmsg(struct kiocb *iocb, stru}}-cleanup_rbuf(sk,copied);+tcp_cleanup_rbuf(sk,copied);if(!sysctl_tcp_low_latency&&tp->ucopy.task==user_recv){/* Install new reader */
@@ -1390,7 +1390,7 @@ skip_copy:*//* Clean up data we have read: This will do ACK frames. */-cleanup_rbuf(sk,copied);+tcp_cleanup_rbuf(sk,copied);TCP_CHECK_TIMER(sk);release_sock(sk);
@@ -1856,7 +1856,7 @@ int tcp_setsockopt(struct sock *sk, int (TCPF_ESTABLISHED|TCPF_CLOSE_WAIT)&&inet_csk_ack_scheduled(sk)){icsk->icsk_ack.pending|=ICSK_ACK_PUSHED;-cleanup_rbuf(sk,1);+tcp_cleanup_rbuf(sk,1);if(!(val&1))icsk->icsk_ack.pingpong=1;}
From: Chris Leech <hidden> Date: 2006-03-03 21:40:47
Any socket recv of less than this ammount will not be offloaded
Signed-off-by: Chris Leech <redacted>
---
include/linux/sysctl.h | 1 +
include/net/tcp.h | 1 +
net/core/user_dma.c | 4 ++++
net/ipv4/sysctl_net_ipv4.c | 10 ++++++++++
4 files changed, 16 insertions(+), 0 deletions(-)
@@ -29,6 +29,9 @@#include<linux/net.h>#include<linux/textsearch.h>#include<net/checksum.h>+#ifdef CONFIG_NET_DMA+#include<linux/dmaengine.h>+#endif#define HAVE_ALLOC_SKB /* For the drivers to know */#define HAVE_ALIGNABLE_SKB /* Ditto 8) */
@@ -285,6 +288,9 @@ struct sk_buff {__u16tc_verd;/* traffic control verdict */#endif#endif+#ifdef CONFIG_NET_DMA+dma_cookie_tdma_cookie;+#endif/* These elements must be at the end, see alloc_skb() for details. */
@@ -813,6 +816,12 @@ static inline void tcp_prequeue_init(strtp->ucopy.len=0;tp->ucopy.memory=0;skb_queue_head_init(&tp->ucopy.prequeue);+#ifdef CONFIG_NET_DMA+tp->ucopy.dma_chan=NULL;+tp->ucopy.wakeup=0;+tp->ucopy.locked_list=NULL;+tp->ucopy.dma_cookie=0;+#endif}/* Packet is added to VJ-style prequeue for processing in process
@@ -0,0 +1,320 @@+/*****************************************************************************+Copyright(c)2004-2006IntelCorporation.Allrightsreserved.+Portionsbasedonnet/core/datagram.candcopyrightedbytheirauthors.++Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+underthetermsoftheGNUGeneralPublicLicenseaspublishedbytheFree+SoftwareFoundation;eitherversion2oftheLicense,or(atyouroption)+anylaterversion.++Thisprogramisdistributedinthehopethatitwillbeuseful,butWITHOUT+ANYWARRANTY;withouteventheimpliedwarrantyofMERCHANTABILITYor+FITNESSFORAPARTICULARPURPOSE.SeetheGNUGeneralPublicLicensefor+moredetails.++YoushouldhavereceivedacopyoftheGNUGeneralPublicLicensealongwith+thisprogram;ifnot,writetotheFreeSoftwareFoundation,Inc.,59+TemplePlace-Suite330,Boston,MA02111-1307,USA.++ThefullGNUGeneralPublicLicenseisincludedinthisdistributioninthe+filecalledLICENSE.+*****************************************************************************/++/*+*ThiscodeallowsthenetstacktomakeuseofaDMAenginefor+*skbtoioveccopies.+*/++#include<linux/dmaengine.h>+#include<linux/pagemap.h>+#include<net/tcp.h> /* for memcpy_toiovec */+#include<asm/io.h>+#include<asm/uaccess.h>++#ifdef CONFIG_DMA_ENGINE++#define NUM_PAGES_SPANNED(start, length) \+((PAGE_ALIGN((unsignedlong)start+length)-\+((unsignedlong)start&PAGE_MASK))>>PAGE_SHIFT)++/*+*Lockdownalltheiovecpagesneededforlenbytes.+*Returnastructdma_locked_listtokeeptrackofpageslockeddown.+*+*Weareallocatingasinglechunkofmemory,andthencarvingitupinto+*3sections,thelatter2whosesizedependsonthenumberofiovecsandthe+*totalnumberofpages,respectively.+*/+intdma_lock_iovec_pages(structiovec*iov,size_tlen,structdma_locked_list+**locked_list)+{+structdma_locked_list*local_list;+structpage**pages;+inti;+intret;++intnr_iovecs=0;+intiovec_len_used=0;+intiovec_pages_used=0;++/* don't lock down non-user-based iovecs */+if(segment_eq(get_fs(),KERNEL_DS)){+*locked_list=NULL;+return0;+}++/* determine how many iovecs/pages there are, up front */+do{+iovec_len_used+=iov[nr_iovecs].iov_len;+iovec_pages_used+=NUM_PAGES_SPANNED(iov[nr_iovecs].iov_base,+iov[nr_iovecs].iov_len);+nr_iovecs++;+}while(iovec_len_used<len);++/* single kmalloc for locked list, page_list[], and the page arrays */+local_list=kmalloc(sizeof(*local_list)++(nr_iovecs*sizeof(structdma_page_list))++(iovec_pages_used*sizeof(structpage*)),GFP_KERNEL);+if(!local_list)+return-ENOMEM;++/* list of pages starts right after the page list array */+pages=(structpage**)&local_list->page_list[nr_iovecs];++/* it's a userspace pointer */+might_sleep();++for(i=0;i<nr_iovecs;i++){+structdma_page_list*page_list=&local_list->page_list[i];++len-=iov[i].iov_len;++if(!access_ok(VERIFY_WRITE,iov[i].iov_base,iov[i].iov_len)){+dma_unlock_iovec_pages(local_list);+return-EFAULT;+}++page_list->nr_pages=NUM_PAGES_SPANNED(iov[i].iov_base,+iov[i].iov_len);+page_list->base_address=iov[i].iov_base;++page_list->pages=pages;+pages+=page_list->nr_pages;++/* lock pages down */+down_read(¤t->mm->mmap_sem);+ret=get_user_pages(+current,+current->mm,+(unsignedlong)iov[i].iov_base,+page_list->nr_pages,+1,+0,+page_list->pages,+NULL);+up_read(¤t->mm->mmap_sem);++if(ret!=page_list->nr_pages){+gotomem_error;+}++local_list->nr_iovecs=i+1;+}++*locked_list=local_list;+return0;++mem_error:+dma_unlock_iovec_pages(local_list);+return-ENOMEM;+}++voiddma_unlock_iovec_pages(structdma_locked_list*locked_list)+{+inti,j;++if(!locked_list)+return;++for(i=0;i<locked_list->nr_iovecs;i++){+structdma_page_list*page_list=&locked_list->page_list[i];+for(j=0;j<page_list->nr_pages;j++){+SetPageDirty(page_list->pages[j]);+page_cache_release(page_list->pages[j]);+}+}++kfree(locked_list);+}++staticdma_cookie_tdma_memcpy_tokerneliovec(structdma_chan*chan,struct+iovec*iov,unsignedchar*kdata,size_tlen)+{+dma_cookie_tdma_cookie=0;++while(len>0){+if(iov->iov_len){+intcopy=min_t(unsignedint,iov->iov_len,len);+dma_cookie=dma_async_memcpy_buf_to_buf(+chan,+iov->iov_base,+kdata,+copy);+kdata+=copy;+len-=copy;+iov->iov_len-=copy;+iov->iov_base+=copy;+}+iov++;+}++returndma_cookie;+}++/*+*Wehavealreadylockeddownthepageswewillbeusingintheiovecs.+*Eachentryiniovarrayhascorrespondingentryinlocked_list->page_list.+*Usingarrayindexingtokeepiov[]andpage_list[]insync.+*Initialelementsiniovarray'siov->iov_lenwillbe0ifalreadycopiedinto+*byanothercall.+*iovarraylengthremainingguaranteedtobebiggerthanlen.+*/+dma_cookie_tdma_memcpy_toiovec(structdma_chan*chan,structiovec*iov,+structdma_locked_list*locked_list,unsignedchar*kdata,size_tlen)+{+intiov_byte_offset;+intcopy;+dma_cookie_tdma_cookie=0;+intiovec_idx;+intpage_idx;++if(!chan)+returnmemcpy_toiovec(iov,kdata,len);++/* -> kernel copies (e.g. smbfs) */+if(!locked_list)+returndma_memcpy_tokerneliovec(chan,iov,kdata,len);++iovec_idx=0;+while(iovec_idx<locked_list->nr_iovecs){+structdma_page_list*page_list;++/* skip already used-up iovecs */+while(!iov[iovec_idx].iov_len)+iovec_idx++;++page_list=&locked_list->page_list[iovec_idx];++iov_byte_offset=((unsignedlong)iov[iovec_idx].iov_base&~PAGE_MASK);+page_idx=(((unsignedlong)iov[iovec_idx].iov_base&PAGE_MASK)+-((unsignedlong)page_list->base_address&PAGE_MASK))>>PAGE_SHIFT;++/* break up copies to not cross page boundary */+while(iov[iovec_idx].iov_len){+copy=min_t(int,PAGE_SIZE-iov_byte_offset,len);+copy=min_t(int,copy,iov[iovec_idx].iov_len);++dma_cookie=dma_async_memcpy_buf_to_pg(chan,+page_list->pages[page_idx],+iov_byte_offset,+kdata,+copy);++len-=copy;+iov[iovec_idx].iov_len-=copy;+iov[iovec_idx].iov_base+=copy;++if(!len)+returndma_cookie;++kdata+=copy;+iov_byte_offset=0;+page_idx++;+}+iovec_idx++;+}++/* really bad if we ever run out of iovecs */+BUG();+return-EFAULT;+}++dma_cookie_tdma_memcpy_pg_toiovec(structdma_chan*chan,structiovec*iov,+structdma_locked_list*locked_list,structpage*page,+unsignedintoffset,size_tlen)+{+intiov_byte_offset;+intcopy;+dma_cookie_tdma_cookie=0;+intiovec_idx;+intpage_idx;+interr;++/* this needs as-yet-unimplemented buf-to-buff, so punt. */+/* TODO: use dma for this */+if(!chan||!locked_list){+u8*vaddr=kmap(page);+err=memcpy_toiovec(iov,vaddr+offset,len);+kunmap(page);+returnerr;+}++iovec_idx=0;+while(iovec_idx<locked_list->nr_iovecs){+structdma_page_list*page_list;++/* skip already used-up iovecs */+while(!iov[iovec_idx].iov_len)+iovec_idx++;++page_list=&locked_list->page_list[iovec_idx];++iov_byte_offset=((unsignedlong)iov[iovec_idx].iov_base&~PAGE_MASK);+page_idx=(((unsignedlong)iov[iovec_idx].iov_base&PAGE_MASK)+-((unsignedlong)page_list->base_address&PAGE_MASK))>>PAGE_SHIFT;++/* break up copies to not cross page boundary */+while(iov[iovec_idx].iov_len){+copy=min_t(int,PAGE_SIZE-iov_byte_offset,len);+copy=min_t(int,copy,iov[iovec_idx].iov_len);++dma_cookie=dma_async_memcpy_pg_to_pg(chan,+page_list->pages[page_idx],+iov_byte_offset,+page,+offset,+copy);++len-=copy;+iov[iovec_idx].iov_len-=copy;+iov[iovec_idx].iov_base+=copy;++if(!len)+returndma_cookie;++offset+=copy;+iov_byte_offset=0;+page_idx++;+}+iovec_idx++;+}++/* really bad if we ever run out of iovecs */+BUG();+return-EFAULT;+}++#else++intdma_lock_iovec_pages(structiovec*iov,size_tlen,structdma_locked_list+**locked_list)+{+*locked_list=NULL;++return0;+}++voiddma_unlock_iovec_pages(structdma_locked_list*locked_list)+{}++#endif
@@ -1109,6 +1112,7 @@ int tcp_recvmsg(struct kiocb *iocb, struinttarget;/* Read at least this many bytes */longtimeo;structtask_struct*user_recv=NULL;+intcopied_early=0;lock_sock(sk);
@@ -1132,6 +1136,12 @@ int tcp_recvmsg(struct kiocb *iocb, strutarget=sock_rcvlowat(sk,flags&MSG_WAITALL,len);+#ifdef CONFIG_NET_DMA+tp->ucopy.dma_chan=NULL;+if((len>sysctl_tcp_dma_copybreak)&&!(flags&MSG_PEEK)&&!sysctl_tcp_low_latency&&__get_cpu_var(softnet_data.net_dma))+dma_lock_iovec_pages(msg->msg_iov,len,&tp->ucopy.locked_list);+#endif+do{structsk_buff*skb;u32offset;
@@ -1273,6 +1283,10 @@ int tcp_recvmsg(struct kiocb *iocb, stru}elsesk_wait_data(sk,&timeo);+#ifdef CONFIG_NET_DMA+tp->ucopy.wakeup=0;+#endif+if(user_recv){intchunk;
@@ -1354,15 +1394,33 @@ skip_copy:if(skb->h.th->fin)gotofound_fin_ok;-if(!(flags&MSG_PEEK))-sk_eat_skb(sk,skb);+if(!(flags&MSG_PEEK)){+if(!copied_early)+sk_eat_skb(sk,skb);+#ifdef CONFIG_NET_DMA+else{+__skb_unlink(skb,&sk->sk_receive_queue);+__skb_queue_tail(&sk->sk_async_wait_queue,skb);+copied_early=0;+}+#endif+}continue;found_fin_ok:/* Process the FIN. */++*seq;-if(!(flags&MSG_PEEK))-sk_eat_skb(sk,skb);+if(!(flags&MSG_PEEK)){+if(!copied_early)+sk_eat_skb(sk,skb);+#ifdef CONFIG_NET_DMA+else{+__skb_unlink(skb,&sk->sk_receive_queue);+__skb_queue_tail(&sk->sk_async_wait_queue,skb);+copied_early=0;+}+#endif+}break;}while(len>0);
@@ -1385,6 +1443,34 @@ skip_copy:tp->ucopy.len=0;}+#ifdef CONFIG_NET_DMA+if(tp->ucopy.dma_chan){+structsk_buff*skb;+dma_cookie_tdone,used;++dma_async_memcpy_issue_pending(tp->ucopy.dma_chan);++while(dma_async_memcpy_complete(tp->ucopy.dma_chan,+tp->ucopy.dma_cookie,&done,+&used)==DMA_IN_PROGRESS){+/* do partial cleanup of sk_async_wait_queue */+while((skb=skb_peek(&sk->sk_async_wait_queue))&&+(dma_async_is_complete(skb->dma_cookie,done,+used)==DMA_SUCCESS)){+__skb_dequeue(&sk->sk_async_wait_queue);+kfree_skb(skb);+}+}++/* Safe to free early-copied skbs now */+__skb_queue_purge(&sk->sk_async_wait_queue);+dma_unlock_iovec_pages(tp->ucopy.locked_list);+dma_chan_put(tp->ucopy.dma_chan);+tp->ucopy.dma_chan=NULL;+tp->ucopy.locked_list=NULL;+}+#endif+/* According to UNIX98, msg_name/msg_namelen are ignored*onconnectedsocket.Iwasjusthappywhenfoundthis8)--ANK*/
@@ -1652,6 +1738,9 @@ int tcp_disconnect(struct sock *sk, int __skb_queue_purge(&sk->sk_receive_queue);sk_stream_writequeue_purge(sk);__skb_queue_purge(&tp->out_of_order_queue);+#ifdef CONFIG_NET_DMA+__skb_queue_purge(&sk->sk_async_wait_queue);+#endifinet->dport=0;
@@ -3901,14 +3904,23 @@ int tcp_rcv_established(struct sock *sk,}}else{inteaten=0;+intcopied_early=0;-if(tp->ucopy.task==current&&-tp->copied_seq==tp->rcv_nxt&&-len-tcp_header_len<=tp->ucopy.len&&-sock_owned_by_user(sk)){-__set_current_state(TASK_RUNNING);+if(tp->copied_seq==tp->rcv_nxt&&+len-tcp_header_len<=tp->ucopy.len){+#ifdef CONFIG_NET_DMA+if(dma_async_try_early_copy(sk,skb,tcp_header_len)){+copied_early=1;+eaten=1;+}+#endif+if(tp->ucopy.task==current&&sock_owned_by_user(sk)&&!copied_early){+__set_current_state(TASK_RUNNING);-if(!tcp_copy_to_iovec(sk,skb,tcp_header_len)){+if(!tcp_copy_to_iovec(sk,skb,tcp_header_len))+eaten=1;+}+if(eaten){/* Predicted packet is in window by definition.*seq==rcv_nxtandrcv_wup<=rcv_nxt.*Hence,checkseq<=rcv_wupreducesto:
@@ -3924,8 +3936,9 @@ int tcp_rcv_established(struct sock *sk,__skb_pull(skb,tcp_header_len);tp->rcv_nxt=TCP_SKB_CB(skb)->end_seq;NET_INC_STATS_BH(LINUX_MIB_TCPHPHITSTOUSER);-eaten=1;}+if(copied_early)+tcp_cleanup_rbuf(sk,skb->len);}if(!eaten){if(tcp_checksum_complete_user(sk,skb))
@@ -3966,6 +3979,11 @@ int tcp_rcv_established(struct sock *sk,__tcp_ack_snd_check(sk,0);no_ack:+#ifdef CONFIG_NET_DMA+if(copied_early)+__skb_queue_tail(&sk->sk_async_wait_queue,skb);+else+#endifif(eaten)__kfree_skb(skb);else
@@ -1292,6 +1305,11 @@ int tcp_v4_destroy_sock(struct sock *sk)/* Cleans up our, hopefully empty, out_of_order_queue. */__skb_queue_purge(&tp->out_of_order_queue);+#ifdef CONFIG_NET_DMA+/* Cleans up our sk_async_wait_queue */+__skb_queue_purge(&sk->sk_async_wait_queue);+#endif+/* Clean prequeue, it must be empty really */__skb_queue_purge(&tp->ucopy.prequeue);
From: Jeff Garzik <hidden> Date: 2006-03-03 22:27:25
Chris Leech wrote:
This patch series is the first full release of the Intel(R) I/O
Acceleration Technology (I/OAT) for Linux. It includes an in kernel API
for offloading memory copies to hardware, a driver for the I/OAT DMA memcpy
engine, and changes to the TCP stack to offload copies of received
networking data to application space.
These changes apply to DaveM's net-2.6.17 tree as of commit
2bd84a93d8bb7192ad8c23ef41008502be1cb603 ([IRDA]: TOIM3232 dongle support)
They are available to pull from
git://198.78.49.142/~cleech/linux-2.6 ioat-2.6.17
There are 8 patches in the series:
1) The memcpy offload APIs and class code
2) The Intel I/OAT DMA driver (ioatdma)
Patch #2 didn't make it. Too big for the list?
Jeff
From: Jeff Garzik <hidden> Date: 2006-03-03 22:45:31
Chris Leech wrote:
quoted
Patch #2 didn't make it. Too big for the list?
Could be, it's the largest of the series. I've attached the gziped
patch. I can try and split this up for the future.
Well, for huge hunks of new code, it sometimes gets silly to split it up.
Once its not in a "reply to email" reviewable form, gzip or URL-to-patch
work just fine.
Jeff
From: Kumar Gala <hidden> Date: 2006-03-03 22:57:58
On Mar 3, 2006, at 3:40 PM, Chris Leech wrote:
This patch series is the first full release of the Intel(R) I/O
Acceleration Technology (I/OAT) for Linux. It includes an in
kernel API
for offloading memory copies to hardware, a driver for the I/OAT
DMA memcpy
engine, and changes to the TCP stack to offload copies of received
networking data to application space.
These changes apply to DaveM's net-2.6.17 tree as of commit
2bd84a93d8bb7192ad8c23ef41008502be1cb603 ([IRDA]: TOIM3232 dongle
support)
They are available to pull from
git://198.78.49.142/~cleech/linux-2.6 ioat-2.6.17
There are 8 patches in the series:
1) The memcpy offload APIs and class code
2) The Intel I/OAT DMA driver (ioatdma)
3) Core networking code to setup networking as a DMA memcpy client
4) Utility functions for sk_buff to iovec offloaded copy
5) Structure changes needed for TCP receive offload
6) Rename cleanup_rbuf to tcp_cleanup_rbuf
7) Add a sysctl to tune the minimum offloaded I/O size for TCP
8) The main TCP receive offload changes
From: Chris Leech <hidden> Date: 2006-03-03 23:32:37
On 3/3/06, Kumar Gala [off-list ref] wrote:
How does this relate to Dan William's ADMA work?
I only became aware of Dan's ADMA work when he posted it last month,
and so far have not made any attempts to merge the I/OAT code with it.
Moving forward, combining these interfaces certainly seems like the
right way to go. I particularly like ADMA's handling of operations
other than just a copy (memset, compare, XOR, CRC).
Chris
On Fri, Mar 03, 2006 at 02:39:22PM -0800, Chris Leech (christopher.leech@intel.com) wrote:
quoted
Patch #2 didn't make it. Too big for the list?
Could be, it's the largest of the series. I've attached the gziped
patch. I can try and split this up for the future.
How can owner of cb_chan->common.device_node be removed?
It looks like that channels are only allocated (without proper error path)
and queued into device->common.channels list in
enumerate_dma_channels() in PCI probe callback and no removing at all, only lockless access.
PCI remove callback only calls dma_async_device_unregister() where only
channel's clients are removed.
From: Jan Engelhardt <hidden> Date: 2006-03-04 18:46:26
This patch series is the first full release of the Intel(R) I/O
Acceleration Technology (I/OAT) for Linux. It includes an in kernel API
for offloading memory copies to hardware, a driver for the I/OAT DMA memcpy
engine, and changes to the TCP stack to offload copies of received
networking data to application space.
Does this buy the normal standard desktop user anything?
Jan Engelhardt
--
This looks like a bug: device is dereferenced after it is potentially
freed.
-ben
--
"Time is of no importance, Mr. President, only life is important."
Don't Email: [off-list ref].
#ifdef is not needed here (try not to put #ifdef in .c files.) I think
a few of your other usages of #ifdef in this file can also be removed
with judicious use of inline functions in a .h file.
thanks,
greg k-h
On Sat, Mar 04, 2006 at 01:41:44PM -0800, David S. Miller (davem@davemloft.net) wrote:
From: Jan Engelhardt <redacted>
Date: Sat, 4 Mar 2006 19:46:22 +0100 (MET)
quoted
Does this buy the normal standard desktop user anything?
Absolutely, it optimizes end-node performance.
It really depends on how it is used.
According to investigation made for kevent based FS AIO reading,
get_user_pages() performange graph looks like sqrt() function
with plato starting on about 64-80 pages on Xeon 2.4Ghz with 1Gb of ram,
while memcopy() is linear, so it can be noticebly slower than
copy_to_user() if get_user_pages() is used aggressively, so userspace
application must reuse the same, already grabbed buffer for maximum
performance, but Intel folks did not provide theirs usage case and any
benchmarks as far as I know.
--
Evgeniy Polyakov
According to investigation made for kevent based FS AIO reading,
get_user_pages() performange graph looks like sqrt() function
with plato starting on about 64-80 pages on Xeon 2.4Ghz with 1Gb of ram,
while memcopy() is linear, so it can be noticebly slower than
copy_to_user() if get_user_pages() is used aggressively, so userspace
application must reuse the same, already grabbed buffer for maximum
performance, but Intel folks did not provide theirs usage case and any
benchmarks as far as I know.
Of course, and programming the DMA controller has overhead
as well. This is why would would not use I/O AT with small
transfer sizes.
static inline all-lower-case functions are much nicer.
+/*
+ * Lock down all the iovec pages needed for len bytes.
+ * Return a struct dma_locked_list to keep track of pages locked down.
+ *
+ * We are allocating a single chunk of memory, and then carving it up into
+ * 3 sections, the latter 2 whose size depends on the number of iovecs and the
+ * total number of pages, respectively.
+ */
+int dma_lock_iovec_pages(struct iovec *iov, size_t len, struct dma_locked_list
+ **locked_list)
Please rename this to dma_pin_iovec_pages(). Locking a page is a quite
different concept from pinning it, and this function doesn't lock any
pages.
+{
+ struct dma_locked_list *local_list;
+ struct page **pages;
+ int i;
+ int ret;
+
+ int nr_iovecs = 0;
+ int iovec_len_used = 0;
+ int iovec_pages_used = 0;
hm, haven't seen that before. Makes sense, I guess.
+ /* determine how many iovecs/pages there are, up front */
+ do {
+ iovec_len_used += iov[nr_iovecs].iov_len;
+ iovec_pages_used += NUM_PAGES_SPANNED(iov[nr_iovecs].iov_base,
+ iov[nr_iovecs].iov_len);
+ nr_iovecs++;
+ } while (iovec_len_used < len);
+
+ /* single kmalloc for locked list, page_list[], and the page arrays */
+ local_list = kmalloc(sizeof(*local_list)
+ + (nr_iovecs * sizeof (struct dma_page_list))
+ + (iovec_pages_used * sizeof (struct page*)), GFP_KERNEL);
What is the upper bound on the size of this allocation?
+ if (!local_list)
+ return -ENOMEM;
+
+ /* list of pages starts right after the page list array */
+ pages = (struct page **) &local_list->page_list[nr_iovecs];
+
+ /* it's a userspace pointer */
+ might_sleep();
kmalloc(GFP_KERNEL) already did that.
+ for (i = 0; i < nr_iovecs; i++) {
+ struct dma_page_list *page_list = &local_list->page_list[i];
+
+ len -= iov[i].iov_len;
+
+ if (!access_ok(VERIFY_WRITE, iov[i].iov_base, iov[i].iov_len)) {
+ dma_unlock_iovec_pages(local_list);
+ return -EFAULT;
+ }
A return statement buried down in the guts of a largeish function isn't
good from a code maintainability POV.
Yes, it has a lot of args. It's nice to add comments like this:
ret = get_user_pages(
current,
current->mm,
(unsigned long) iov[i].iov_base,
page_list->nr_pages,
1, /* write */
0, /* force */
page_list->pages,
NULL);
Suggest you change this function to return locked_list, or an IS_ERR value
on error.
+void dma_unlock_iovec_pages(struct dma_locked_list *locked_list)
+{
+ int i, j;
+
+ if (!locked_list)
+ return;
+
+ for (i = 0; i < locked_list->nr_iovecs; i++) {
+ struct dma_page_list *page_list = &locked_list->page_list[i];
+ for (j = 0; j < page_list->nr_pages; j++) {
+ SetPageDirty(page_list->pages[j]);
+ page_cache_release(page_list->pages[j]);
+ }
+ }
+
+ kfree(locked_list);
+}
SetPageDirty() is very wrong. It fails to mark pagecache pages as dirty in
the radix tree so they won't get written back.
You'll need to use set_page_dirty_lock() here or, if you happen to have
protected the inode which backs this potential mmap (really the
address_space) from reclaim then set_page_dirty() will work. Probably
it'll be set_page_dirty_lock().
If this is called from cant-sleep context then things get ugly. If it's
called from interrupt context then moreso. See fs/direct-io.c,
bio_set_pages_dirty(), bio_check_pages_dirty(), etc.
I don't see a check for "did we write to user pages" here. Because we
don't need to dirty the pages if we were reading them (transmitting from
userspace).
But given that dma_lock_iovec_pages() is only set up for writing to
userspace I guess this code is implicitly receive-only. It's hard to tell
when the description, is, like the code comments, so scant.
+{
+ int iov_byte_offset;
+ int copy;
+ dma_cookie_t dma_cookie = 0;
+ int iovec_idx;
+ int page_idx;
+ int err;
+
+ /* this needs as-yet-unimplemented buf-to-buff, so punt. */
+ /* TODO: use dma for this */
+ if (!chan || !locked_list) {
Really you should rename locked_list to pinned_list throughout, and
dma_locked_list to dma_pinned_list.
+#ifdef CONFIG_NET_DMA
+
+/**
+ * dma_skb_copy_datagram_iovec - Copy a datagram to an iovec.
+ * @skb - buffer to copy
+ * @offset - offset in the buffer to start copying from
+ * @iovec - io vector to copy to
+ * @len - amount of data to copy from buffer to iovec
+ * @locked_list - locked iovec buffer data
+ *
+ * Note: the iovec is modified during the copy.
Modifying the caller's iovec is a bit rude. Hard to avoid, I guess.
+ */
+int dma_skb_copy_datagram_iovec(struct dma_chan *chan,
+ struct sk_buff *skb, int offset, struct iovec *to,
+ size_t len, struct dma_locked_list *locked_list)
+{
+ int start = skb_headlen(skb);
+ int i, copy = start - offset;
+ dma_cookie_t cookie = 0;
+
+ /* Copy header. */
+ if (copy > 0) {
+ if (copy > len)
+ copy = len;
+ if ((cookie = dma_memcpy_toiovec(chan, to, locked_list,
+ skb->data + offset, copy)) < 0)
+ goto fault;
+ if ((len -= copy) == 0)
+ goto end;
Please avoid
if ((lhs = rhs))
constructs. Instead do
lhs = rhs;
if (lhs)
(entire patchset - there are quite a lot)
+ offset += copy;
+ }
+
+ /* Copy paged appendix. Hmm... why does this look so complicated? */
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ int end;
+
+ BUG_TRAP(start <= offset + len);
<wonders why BUG_TRAP still exists>
+ if ((copy = end - offset) > 0) {
...
+ if (!(len -= copy))
...
+ if ((copy = end - offset) > 0) {
...
+ if ((len -= copy) == 0)
Please try to fit code into 80 columns.
That's decimal 80 ;)
quoted hunk
@@ -1328,13 +1342,39 @@ do_prequeue: } if (!(flags & MSG_TRUNC)) {- err = skb_copy_datagram_iovec(skb, offset,- msg->msg_iov, used);- if (err) {- /* Exception. Bailout! */- if (!copied)- copied = -EFAULT;- break;+#ifdef CONFIG_NET_DMA+ if (!tp->ucopy.dma_chan && tp->ucopy.locked_list)+ tp->ucopy.dma_chan = get_softnet_dma();++ if (tp->ucopy.dma_chan) {+ tp->ucopy.dma_cookie = dma_skb_copy_datagram_iovec(+ tp->ucopy.dma_chan, skb, offset,+ msg->msg_iov, used,+ tp->ucopy.locked_list);++ if (tp->ucopy.dma_cookie < 0) {++ printk(KERN_ALERT "dma_cookie < 0\n");++ /* Exception. Bailout! */+ if (!copied)+ copied = -EFAULT;+ break;+ }+ if ((offset + used) == skb->len)+ copied_early = 1;+
Consider trimming some of those blank lines. I don't think they add any
value?
+ } else
+#endif
+ {
These games with ifdefs and else statements aren't at all pleasant.
Sometimes they're hard to avoid, but you'll probably find that some code
rearrangemnt (in a preceding patch) makes it easier. Like, split this
function into several.
quoted hunk
@@ -1354,15 +1394,33 @@ skip_copy: if (skb->h.th->fin) goto found_fin_ok;- if (!(flags & MSG_PEEK))- sk_eat_skb(sk, skb);+ if (!(flags & MSG_PEEK)) {+ if (!copied_early)+ sk_eat_skb(sk, skb);+#ifdef CONFIG_NET_DMA+ else {+ __skb_unlink(skb, &sk->sk_receive_queue);+ __skb_queue_tail(&sk->sk_async_wait_queue, skb);+ copied_early = 0;+ }+#endif
From: Andrew Morton <hidden> Date: 2006-03-05 08:11:19
"Chris Leech" [off-list ref] wrote:
quoted
Patch #2 didn't make it. Too big for the list?
Could be, it's the largest of the series. I've attached the gziped
patch. I can try and split this up for the future.
..
[I/OAT] Driver for the Intel(R) I/OAT DMA engine
Adds a new ioatdma driver
...
+struct cb_pci_pmcap_register {
+ uint32_t capid:8; /* RO: 01h */
+ uint32_t nxtcapptr:8;
+ uint32_t version:3; /* RO: 010b */
+ uint32_t pmeclk:1; /* RO: 0b */
+ uint32_t reserved:1; /* RV: 0b */
+ uint32_t dsi:1; /* RO: 0b */
+ uint32_t aux_current:3; /* RO: 000b */
+ uint32_t d1_support:1; /* RO: 0b */
+ uint32_t d2_support:1; /* RO: 0b */
+ uint32_t pme_support:5; /* RO: 11001b */
+};
This maps onto hardware registers? No big-endian plans in Intel's future? ;)
I have a vague feeling that gcc changed its layout of bitfields many years
ago. I guess we're fairly safe against that. Presumably gcc and icc use the
same layout?
Still. It's a bit of a concern, but I guess we can worry about that if it
happens.
These are fairly generic-sounding names. In fact the as-yet-unmerged tiacx
wireless driver is already using these, privately to
drivers/net/wireless/tiacx/pci.c.
There's a mix of styles here. I don't think the space after the asterisk does
anything useful, and it could be argued that it's incorrect (or misleading)
wrt C declaration semantics.
GFP_ATOMIC is to be avoided if at all possible. It stresses the memory system
and can easily fail under load.
From my reading, two of the callers could trivially call this function outside
spin_lock_bh() and the third could perhaps do so with a little work. You
could at least fix up two of those callers, and pass in the gfp_flags.
<wonders why the heck dma_pool_alloc() uses SLAB_ATOMIC when the caller's
passing in the gfp_flags>
+/* returns the actual number of allocated descriptors */
+static int cb_dma_alloc_chan_resources(struct dma_chan *chan)
+{
...
+ /* Allocate descriptors */
+ spin_lock_bh(&cb_chan->desc_lock);
+ for (i = 0; i < INITIAL_CB_DESC_COUNT; i++) {
+ desc = cb_dma_alloc_descriptor(cb_chan);
+ if (!desc) {
+ printk(KERN_ERR "CB: Only %d initial descriptors\n", i);
+ break;
+ }
+ list_add_tail(&desc->node, &cb_chan->free_desc);
+ }
+ spin_unlock_bh(&cb_chan->desc_lock);
What's going on here? Lock ranking problems? spin_trylock() in
non-infrastructural code is a bit of a red flag.
Whatever the reason, it needs a comment in there please. That comment should
also explain why simply baling out is acceptable.
The __get_cpu_var() here will run smp_processor_id() from preemptible
context. You'll get a big warning if the correct debug options are set.
The reason for this is that preemption could cause this code to hop between
CPUs.
Please always test code with all debug options enabled and with full kernel
preemption.
These are fairly generic-sounding names. In fact the as-yet-unmerged tiacx
wireless driver is already using these, privately to
drivers/net/wireless/tiacx/pci.c.
Do we in general discourage duplicate symbols even if they are static?
[ppc64, allmodconfig]
$> nm vmlinux | fgrep ' t ' | awk '{print $3}' | sort | uniq -dc
2 .add_bridge
2 .base_probe
2 .c_next
2 .c_start
2 .c_stop
3 .cpu_callback
2 .default_open
2 .default_read_file
2 .default_write_file
2 .dev_ifsioc
2 .do_open
4 .dst_output
2 .dump_seek
2 .dump_write
2 .elf_core_dump
2 .elf_map
2 .exact_lock
2 .exact_match
2 .exit_elf_binfmt
2 .fill_note
2 .fill_prstatus
2 .fillonedir
2 .fini
2 .fixup_one_level_bus_range
5 .init
8 .init_once
3 .iommu_bus_setup_null
3 .iommu_dev_setup_null
2 .klist_devices_get
2 .klist_devices_put
2 .load_elf_binary
2 .load_elf_interp
2 .load_elf_library
3 .m_next
3 .m_start
3 .m_stop
2 .maydump
3 .modalias_show
2 .next_device
3 .notesize
2 .padzero
2 .raw_ioctl
2 .s_next
2 .s_show
2 .s_start
2 .s_stop
2 .seq_next
2 .seq_show
2 .seq_start
2 .seq_stop
2 .set_brk
2 .setkey
2 .state_show
2 .state_store
2 .store_uevent
2 .u3_ht_cfg_access
2 .u3_ht_read_config
2 .u3_ht_write_config
2 .writenote
3 __initcall_init
2 __setup_netdev_boot_setup
2 __setup_str_netdev_boot_setup
If I did a make allyesconfig the result looks much more scary.
Sam
From: Andrew Morton <hidden> Date: 2006-03-05 09:20:42
Sam Ravnborg [off-list ref] wrote:
On Sun, Mar 05, 2006 at 12:09:33AM -0800, Andrew Morton wrote:
> > +
> > +static inline u8 read_reg8(struct cb_device *device, unsigned int offset)
> > +{
> > + return readb(device->reg_base + offset);
> > +}
>
> These are fairly generic-sounding names. In fact the as-yet-unmerged tiacx
> wireless driver is already using these, privately to
> drivers/net/wireless/tiacx/pci.c.
Do we in general discourage duplicate symbols even if they are static?
Well, it's a bit irritating that it confuses ctags. But in this case, one
set is in a header file so the risk of collisions is much-increased.
From: "David S. Miller" <davem@davemloft.net> Date: 2006-03-05 10:27:28
From: Andrew Morton <redacted>
Date: Sun, 5 Mar 2006 00:45:34 -0800
The __get_cpu_var() here will run smp_processor_id() from preemptible
context. You'll get a big warning if the correct debug options are set.
The reason for this is that preemption could cause this code to hop between
CPUs.
Please always test code with all debug options enabled and with full kernel
preemption.
To be fair that warning doesn't trigger on some platforms, such as
sparc64 where the __get_cpu_var() implementation simply takes the
value from a fixed cpu register and doesn't do the debugging check.
Sparc64 should add the check when debugging options are enabled, for
sure, but the point is that it may not entirely be the tester's fault.
:-)
On Sat, Mar 04, 2006 at 01:41:44PM -0800, David S. Miller (davem@davemloft.net) wrote:
quoted
From: Jan Engelhardt <redacted>
Date: Sat, 4 Mar 2006 19:46:22 +0100 (MET)
quoted
Does this buy the normal standard desktop user anything?
Absolutely, it optimizes end-node performance.
It really depends on how it is used.
According to investigation made for kevent based FS AIO reading,
get_user_pages() performange graph looks like sqrt() function
Hmm, so I should resurrect my user page table walker abstraction?
There I would hand each page to a "recording" function, which
can drop the page from the collection or coalesce it in the collector
if your scatter gather implementation allows it.
Regards
Ingo Oeser
From: Chris Leech <hidden> Date: 2006-03-06 19:29:00
#ifdef is not needed here (try not to put #ifdef in .c files.) I think
a few of your other usages of #ifdef in this file can also be removed
with judicious use of inline functions in a .h file.
ACK on all the ifdef comments. I may have gone a little ifdef crazy
making sure I could get to a zero impact state with these patches
applied but CONFIG_NET_DMA turned off. I'll get these cleaned up.
- Chris
The __get_cpu_var() here will run smp_processor_id() from preemptible
context. You'll get a big warning if the correct debug options are set.
The reason for this is that preemption could cause this code to hop between
CPUs.
I've been playing with different models of where to select which DMA
channel to use in order to reduce cache thrash and lock contention in
the driver. It's not a clean per-cpu issue because per I/O there are
potentially operations happening in both the process syscall and the
netrx softirq context.
Right now the code delays selection of a DMA channel until the first
offload copy is ready to go, so the __get_cpu_var() you point out is
just checking to see if any hardware exists for I/OAT at this point
before doing the page pinning. Before anything is done with the
channel the per-cpu pointer is re-read safely with preemption disabled
and a reference count is incremented.
- Chris
This looks like a bug: device is dereferenced after it is potentially
freed.
Actually, this is where the code is waiting to make sure it's safe to
free device. The release function for the kref completes
device->done. Each of the devices channels holds a reference to the
device. When a device is unregistered it's channels are removed from
the clients, which hold a reference for each outstanding transaction.
When all the outstanding transactions complete, the channels kref goes
to 0, and the reference to the device is dropped. When the device
kref goes to 0 the completion is set, and it's now safe to free the
memory for the device and channel structures.
I have a writeup of the locking and reference counting that I'll
finish and add in as a big comment to the code.
-Chris
From: Chris Leech <hidden> Date: 2006-03-06 19:56:30
On 3/5/06, Andrew Morton [off-list ref] wrote:
Sam Ravnborg [off-list ref] wrote:
quoted
On Sun, Mar 05, 2006 at 12:09:33AM -0800, Andrew Morton wrote:
> > +
> > +static inline u8 read_reg8(struct cb_device *device, unsigned int offset)
> > +{
> > + return readb(device->reg_base + offset);
> > +}
>
> These are fairly generic-sounding names. In fact the as-yet-unmerged tiacx
> wireless driver is already using these, privately to
> drivers/net/wireless/tiacx/pci.c.
Do we in general discourage duplicate symbols even if they are static?
Well, it's a bit irritating that it confuses ctags. But in this case, one
set is in a header file so the risk of collisions is much-increased.
They're in a header file that's specific to a single driver, so I
don't see where a conflict would occur. But I didn't think about
ctags, and these can easily be prefixed so I'll go ahead and change
them.
- Chris
On Mon, Mar 06, 2006 at 06:44:07PM +0100, Ingo Oeser (netdev@axxeo.de) wrote:
Evgeniy Polyakov wrote:
quoted
On Sat, Mar 04, 2006 at 01:41:44PM -0800, David S. Miller (davem@davemloft.net) wrote:
quoted
From: Jan Engelhardt <redacted>
Date: Sat, 4 Mar 2006 19:46:22 +0100 (MET)
quoted
Does this buy the normal standard desktop user anything?
Absolutely, it optimizes end-node performance.
It really depends on how it is used.
According to investigation made for kevent based FS AIO reading,
get_user_pages() performange graph looks like sqrt() function
Hmm, so I should resurrect my user page table walker abstraction?
There I would hand each page to a "recording" function, which
can drop the page from the collection or coalesce it in the collector
if your scatter gather implementation allows it.
It depends on where performance growth is stopped.
From the first glance it does not look like find_extend_vma(),
probably follow_page() fault and thus __handle_mm_fault().
I can not say actually, but if it is true and performance growth is
stopped due to increased number of faults and it's processing,
your approach will hit this problem too, doesn't it?
On Mon, Mar 06, 2006 at 06:44:07PM +0100, Ingo Oeser (netdev@axxeo.de) wrote:
quoted
Hmm, so I should resurrect my user page table walker abstraction?
There I would hand each page to a "recording" function, which
can drop the page from the collection or coalesce it in the collector
if your scatter gather implementation allows it.
It depends on where performance growth is stopped.
From the first glance it does not look like find_extend_vma(),
probably follow_page() fault and thus __handle_mm_fault().
I can not say actually, but if it is true and performance growth is
stopped due to increased number of faults and it's processing,
your approach will hit this problem too, doesn't it?
My approach reduced the number of loops performed and number
of memory needed at the expense of doing more work in the main
loop of get_user_pages.
This was mitigated for the common case of getting just one page by
providing a get_one_user_page() function.
The whole problem, why we need such multiple loops is that we have
no common container object for "IO vector + additional data".
So we always do a loop working over the vector returned by
get_user_pages() all the time. The bigger that vector,
the bigger the impact.
Maybe sth. as simple as providing get_user_pages() with some offset_of
and container_of hackery will work these days without the disadvantages
my old get_user_pages() work had.
The idea is, that you'll provide a vector (like arguments to calloc) and two
offsets: One for the page to store within the offset and one for the vma
to store.
If the offset has a special value (e.g MAX_LONG) you don't store there at all.
But if the performance problem really is get_user_pages() itself
(and not its callers), then my approach won't help at all.
Regards
Ingo Oeser
On Tue, Mar 07, 2006 at 10:43:59AM +0100, Ingo Oeser (netdev@axxeo.de) wrote:
Evgeniy Polyakov wrote:
quoted
On Mon, Mar 06, 2006 at 06:44:07PM +0100, Ingo Oeser (netdev@axxeo.de) wrote:
quoted
Hmm, so I should resurrect my user page table walker abstraction?
There I would hand each page to a "recording" function, which
can drop the page from the collection or coalesce it in the collector
if your scatter gather implementation allows it.
It depends on where performance growth is stopped.
From the first glance it does not look like find_extend_vma(),
probably follow_page() fault and thus __handle_mm_fault().
I can not say actually, but if it is true and performance growth is
stopped due to increased number of faults and it's processing,
your approach will hit this problem too, doesn't it?
My approach reduced the number of loops performed and number
of memory needed at the expense of doing more work in the main
loop of get_user_pages.
This was mitigated for the common case of getting just one page by
providing a get_one_user_page() function.
The whole problem, why we need such multiple loops is that we have
no common container object for "IO vector + additional data".
So we always do a loop working over the vector returned by
get_user_pages() all the time. The bigger that vector,
the bigger the impact.
Maybe sth. as simple as providing get_user_pages() with some offset_of
and container_of hackery will work these days without the disadvantages
my old get_user_pages() work had.
The idea is, that you'll provide a vector (like arguments to calloc) and two
offsets: One for the page to store within the offset and one for the vma
to store.
If the offset has a special value (e.g MAX_LONG) you don't store there at all.
You still need to find VMA in one loop, and run through it's(mm_structu) pages in
second loop.
But if the performance problem really is get_user_pages() itself
(and not its callers), then my approach won't help at all.
It looks so.
My test pseudocode is following:
fget_light();
igrab();
kzalloc(number_of_pages * sizeof(void *));
get_user_pages(number_of_pages);
... undo ...
I've attached two graphs of performance with and without
get_user_pages(), it is get_user_pages.png and kmalloc.png.
Vertical axis is number of Mbytes per second thrown through above code,
horizontal one is number of pages in each run.