From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-DB5-obe.outbound.protection.outlook.com (mail-eopbgr40088.outbound.protection.outlook.com [40.107.4.88]) by dpdk.org (Postfix) with ESMTP id 2EF9828EE; Mon, 22 May 2017 12:15:25 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=co1uOFuHlZEdHhEoDhPDpEg3Ezb67GydPsU9RsS9//M=; b=IQ49Dwr+ScHaspH0pVohwjWpWmqiXecMXmLSfnXhaidze23fcChEAqpOZQkidJkGPYusRy2pedI+0z3HxL5T/gHWiS/H4vKVskLU3b6CBXw4NFk3LEyXhzYREKm2+UXx+7VW7UXoM5QAdnhQE1ze9hHSPmq/lv27Qtm/V3VQfa8= Received: from AM4PR05MB1505.eurprd05.prod.outlook.com (10.164.79.147) by AM4PR0501MB2755.eurprd05.prod.outlook.com (10.172.216.11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1101.14; Mon, 22 May 2017 10:15:23 +0000 Received: from AM4PR05MB1505.eurprd05.prod.outlook.com ([fe80::3880:1147:d0d0:cdbe]) by AM4PR05MB1505.eurprd05.prod.outlook.com ([fe80::3880:1147:d0d0:cdbe%14]) with mapi id 15.01.1101.019; Mon, 22 May 2017 10:15:23 +0000 From: Shahaf Shuler To: Tiwei Bie , "dev@dpdk.org" , =?iso-8859-1?Q?N=E9lio_Laranjeiro?= , "Adrien Mazarguil" , Olga Shern CC: "bruce.richardson@intel.com" , "stable@dpdk.org" Thread-Topic: [dpdk-stable] [PATCH v2 2/2] contigmem: don't zero the pages during each mmap Thread-Index: AQHS0twOt0O4zQDbGkOQZ9a/J+MVIKIAIqzQ Date: Mon, 22 May 2017 10:15:23 +0000 Message-ID: References: <20170509084707.29949-1-tiwei.bie@intel.com> <20170522090349.82175-1-tiwei.bie@intel.com> <20170522090349.82175-3-tiwei.bie@intel.com> In-Reply-To: <20170522090349.82175-3-tiwei.bie@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=mellanox.com; x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM4PR0501MB2755; 7:IvxdX8Ko6NP8tz6Aux+HuXiZ201tBXzYdVXEJZ641E5ovJUaPCwlYmFPpDkT+lRW3NcTgraIh5d2DgTGRbI+RJp/lHTfNesDsN21Wcg1iJwHrQpEjWLgDkIRvUfHXV6qZ+I1RAPDEUe2o8js4kKxvEQWn9q8dVtc2P/cHgSCmZSYMYJbn3OJfcdJvm7+RNSdd0Ilfrb+pIhV3g/bSRsle55JE5138hEPYqrk803gZROoH6fhe/umxT633LVAc7HrZ9VFXUZA8DPL5euE9VlGxmkVeP8mrS9l8TQR4luKgfxXm73hRdKUIhqNJiaeJ4Dx29I7V77JvOGqXmLk9pZyvQ== x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-ms-traffictypediagnostic: AM4PR0501MB2755: x-ms-office365-filtering-correlation-id: 32c667a9-e571-4adc-0a49-08d4a0fb7566 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(2017030254075)(48565401081)(201703131423075)(201703031133081); SRVR:AM4PR0501MB2755; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(788757137089)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040450)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(93006095)(93001095)(6055026)(6041248)(20161123558100)(20161123564025)(20161123555025)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123560025)(6072148); SRVR:AM4PR0501MB2755; BCL:0; PCL:0; RULEID:; SRVR:AM4PR0501MB2755; x-forefront-prvs: 03152A99FF x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(39860400002)(39840400002)(39450400003)(39410400002)(39850400002)(39400400002)(377454003)(6436002)(99286003)(54906002)(55016002)(25786009)(4326008)(9686003)(74316002)(305945005)(189998001)(7736002)(66066001)(53936002)(5660300001)(5250100002)(50986999)(6246003)(7696004)(86362001)(76176999)(54356999)(38730400002)(6636002)(102836003)(3846002)(478600001)(229853002)(6116002)(2950100002)(2900100001)(81166006)(8936002)(3280700002)(2906002)(3660700001)(33656002); DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR0501MB2755; H:AM4PR05MB1505.eurprd05.prod.outlook.com; FPR:; SPF:None; MLV:sfv; LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-originalarrivaltime: 22 May 2017 10:15:23.0752 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0501MB2755 Subject: Re: [dpdk-stable] [PATCH v2 2/2] contigmem: don't zero the pages during each mmap X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 May 2017 10:15:25 -0000 N=E9lio, related to the issue you saw with secondary process support? --Shahaf Monday, May 22, 2017 12:04 PM, Tiwei Bie: > Don't zero the pages during each mmap. Instead, only zero the pages when > they are not already mmapped. Otherwise, the multi-process support will b= e > broken, as the pages will be zeroed when secondary processes map the > memory. Besides, track the open and mmap operations on the cdev, and > prevent the module from being unloaded when it is still in use. >=20 > Fixes: 82f931805506 ("contigmem: zero all pages during mmap") > Cc: stable@dpdk.org >=20 > Signed-off-by: Tiwei Bie > Acked-by: Bruce Richardson > --- > lib/librte_eal/bsdapp/contigmem/contigmem.c | 183 > ++++++++++++++++++++++++---- > 1 file changed, 157 insertions(+), 26 deletions(-) >=20 > diff --git a/lib/librte_eal/bsdapp/contigmem/contigmem.c > b/lib/librte_eal/bsdapp/contigmem/contigmem.c > index 03e3e8def..b201d7bf4 100644 > --- a/lib/librte_eal/bsdapp/contigmem/contigmem.c > +++ b/lib/librte_eal/bsdapp/contigmem/contigmem.c > @@ -50,24 +50,37 @@ __FBSDID("$FreeBSD$"); >=20 > #include > #include > +#include > #include > #include > #include > +#include > + > +struct contigmem_buffer { > + void *addr; > + int refcnt; > + struct mtx mtx; > +}; > + > +struct contigmem_vm_handle { > + int buffer_index; > +}; >=20 > static int contigmem_load(void); > static int contigmem_unload(void); > static int contigmem_physaddr(SYSCTL_HANDLER_ARGS); >=20 > -static d_mmap_t contigmem_mmap; > static d_mmap_single_t contigmem_mmap_single; > static d_open_t contigmem_open; > +static d_close_t contigmem_close; >=20 > static int contigmem_num_buffers =3D > RTE_CONTIGMEM_DEFAULT_NUM_BUFS; > static int64_t contigmem_buffer_size =3D > RTE_CONTIGMEM_DEFAULT_BUF_SIZE; >=20 > static eventhandler_tag contigmem_eh_tag; > -static void > *contigmem_buffers[RTE_CONTIGMEM_MAX_NUM_BUFS]; > +static struct contigmem_buffer > +contigmem_buffers[RTE_CONTIGMEM_MAX_NUM_BUFS]; > static struct cdev *contigmem_cdev =3D NULL; > +static int contigmem_refcnt; >=20 > TUNABLE_INT("hw.contigmem.num_buffers", &contigmem_num_buffers); > TUNABLE_QUAD("hw.contigmem.buffer_size", &contigmem_buffer_size); > @@ -78,6 +91,8 @@ SYSCTL_INT(_hw_contigmem, OID_AUTO, > num_buffers, CTLFLAG_RD, > &contigmem_num_buffers, 0, "Number of contigmem buffers > allocated"); SYSCTL_QUAD(_hw_contigmem, OID_AUTO, buffer_size, > CTLFLAG_RD, > &contigmem_buffer_size, 0, "Size of each contiguous buffer"); > +SYSCTL_INT(_hw_contigmem, OID_AUTO, num_references, CTLFLAG_RD, > + &contigmem_refcnt, 0, "Number of references to contigmem"); >=20 > static SYSCTL_NODE(_hw_contigmem, OID_AUTO, physaddr, CTLFLAG_RD, > 0, > "physaddr"); > @@ -114,9 +129,10 @@ MODULE_VERSION(contigmem, 1); static struct > cdevsw contigmem_ops =3D { > .d_name =3D "contigmem", > .d_version =3D D_VERSION, > - .d_mmap =3D contigmem_mmap, > + .d_flags =3D D_TRACKCLOSE, > .d_mmap_single =3D contigmem_mmap_single, > .d_open =3D contigmem_open, > + .d_close =3D contigmem_close, > }; >=20 > static int > @@ -124,6 +140,7 @@ contigmem_load() > { > char index_string[8], description[32]; > int i, error =3D 0; > + void *addr; >=20 > if (contigmem_num_buffers > > RTE_CONTIGMEM_MAX_NUM_BUFS) { > printf("%d buffers requested is greater than %d allowed\n", > @@ -141,18 +158,20 @@ contigmem_load() > } >=20 > for (i =3D 0; i < contigmem_num_buffers; i++) { > - contigmem_buffers[i] =3D > - contigmalloc(contigmem_buffer_size, > M_CONTIGMEM, M_ZERO, 0, > - BUS_SPACE_MAXADDR, contigmem_buffer_size, 0); > - > - if (contigmem_buffers[i] =3D=3D NULL) { > + addr =3D contigmalloc(contigmem_buffer_size, > M_CONTIGMEM, M_ZERO, > + 0, BUS_SPACE_MAXADDR, contigmem_buffer_size, > 0); > + if (addr =3D=3D NULL) { > printf("contigmalloc failed for buffer %d\n", i); > error =3D ENOMEM; > goto error; > } >=20 > - printf("%2u: virt=3D%p phys=3D%p\n", i, contigmem_buffers[i], > - (void > *)pmap_kextract((vm_offset_t)contigmem_buffers[i])); > + printf("%2u: virt=3D%p phys=3D%p\n", i, addr, > + (void *)pmap_kextract((vm_offset_t)addr)); > + > + mtx_init(&contigmem_buffers[i].mtx, "contigmem", NULL, > MTX_DEF); > + contigmem_buffers[i].addr =3D addr; > + contigmem_buffers[i].refcnt =3D 0; >=20 > snprintf(index_string, sizeof(index_string), "%d", i); > snprintf(description, sizeof(description), @@ -170,10 +189,13 > @@ contigmem_load() > return 0; >=20 > error: > - for (i =3D 0; i < contigmem_num_buffers; i++) > - if (contigmem_buffers[i] !=3D NULL) > - contigfree(contigmem_buffers[i], > contigmem_buffer_size, > - M_CONTIGMEM); > + for (i =3D 0; i < contigmem_num_buffers; i++) { > + if (contigmem_buffers[i].addr !=3D NULL) > + contigfree(contigmem_buffers[i].addr, > + contigmem_buffer_size, M_CONTIGMEM); > + if (mtx_initialized(&contigmem_buffers[i].mtx)) > + mtx_destroy(&contigmem_buffers[i].mtx); > + } >=20 > return error; > } > @@ -183,16 +205,22 @@ contigmem_unload() { > int i; >=20 > + if (contigmem_refcnt > 0) > + return EBUSY; > + > if (contigmem_cdev !=3D NULL) > destroy_dev(contigmem_cdev); >=20 > if (contigmem_eh_tag !=3D NULL) > EVENTHANDLER_DEREGISTER(process_exit, > contigmem_eh_tag); >=20 > - for (i =3D 0; i < RTE_CONTIGMEM_MAX_NUM_BUFS; i++) > - if (contigmem_buffers[i] !=3D NULL) > - contigfree(contigmem_buffers[i], > contigmem_buffer_size, > - M_CONTIGMEM); > + for (i =3D 0; i < RTE_CONTIGMEM_MAX_NUM_BUFS; i++) { > + if (contigmem_buffers[i].addr !=3D NULL) > + contigfree(contigmem_buffers[i].addr, > + contigmem_buffer_size, M_CONTIGMEM); > + if (mtx_initialized(&contigmem_buffers[i].mtx)) > + mtx_destroy(&contigmem_buffers[i].mtx); > + } >=20 > return 0; > } > @@ -203,7 +231,7 @@ contigmem_physaddr(SYSCTL_HANDLER_ARGS) > uint64_t physaddr; > int index =3D (int)(uintptr_t)arg1; >=20 > - physaddr =3D (uint64_t)vtophys(contigmem_buffers[index]); > + physaddr =3D (uint64_t)vtophys(contigmem_buffers[index].addr); > return sysctl_handle_64(oidp, &physaddr, 0, req); } >=20 > @@ -211,22 +239,118 @@ static int > contigmem_open(struct cdev *cdev, int fflags, int devtype, > struct thread *td) > { > + > + atomic_add_int(&contigmem_refcnt, 1); > + > + return 0; > +} > + > +static int > +contigmem_close(struct cdev *cdev, int fflags, int devtype, > + struct thread *td) > +{ > + > + atomic_subtract_int(&contigmem_refcnt, 1); > + > return 0; > } >=20 > static int > -contigmem_mmap(struct cdev *cdev, vm_ooffset_t offset, vm_paddr_t > *paddr, > - int prot, vm_memattr_t *memattr) > +contigmem_cdev_pager_ctor(void *handle, vm_ooffset_t size, vm_prot_t > prot, > + vm_ooffset_t foff, struct ucred *cred, u_short *color) > { > + struct contigmem_vm_handle *vmh =3D handle; > + struct contigmem_buffer *buf; > + > + buf =3D &contigmem_buffers[vmh->buffer_index]; > + > + atomic_add_int(&contigmem_refcnt, 1); > + > + mtx_lock(&buf->mtx); > + if (buf->refcnt =3D=3D 0) > + memset(buf->addr, 0, contigmem_buffer_size); > + buf->refcnt++; > + mtx_unlock(&buf->mtx); >=20 > - *paddr =3D offset; > return 0; > } >=20 > +static void > +contigmem_cdev_pager_dtor(void *handle) { > + struct contigmem_vm_handle *vmh =3D handle; > + struct contigmem_buffer *buf; > + > + buf =3D &contigmem_buffers[vmh->buffer_index]; > + > + mtx_lock(&buf->mtx); > + buf->refcnt--; > + mtx_unlock(&buf->mtx); > + > + free(vmh, M_CONTIGMEM); > + > + atomic_subtract_int(&contigmem_refcnt, 1); } > + > +static int > +contigmem_cdev_pager_fault(vm_object_t object, vm_ooffset_t offset, > int prot, > + vm_page_t *mres) > +{ > + vm_paddr_t paddr; > + vm_page_t m_paddr, page; > + vm_memattr_t memattr, memattr1; > + > + memattr =3D object->memattr; > + > + VM_OBJECT_WUNLOCK(object); > + > + paddr =3D offset; > + > + m_paddr =3D vm_phys_paddr_to_vm_page(paddr); > + if (m_paddr !=3D NULL) { > + memattr1 =3D pmap_page_get_memattr(m_paddr); > + if (memattr1 !=3D memattr) > + memattr =3D memattr1; > + } > + > + if (((*mres)->flags & PG_FICTITIOUS) !=3D 0) { > + /* > + * If the passed in result page is a fake page, update it with > + * the new physical address. > + */ > + page =3D *mres; > + VM_OBJECT_WLOCK(object); > + vm_page_updatefake(page, paddr, memattr); > + } else { > + /* > + * Replace the passed in reqpage page with our own fake > page and > + * free up the all of the original pages. > + */ > + page =3D vm_page_getfake(paddr, memattr); > + VM_OBJECT_WLOCK(object); > + vm_page_replace_checked(page, object, (*mres)->pindex, > *mres); > + vm_page_lock(*mres); > + vm_page_free(*mres); > + vm_page_unlock(*mres); > + *mres =3D page; > + } > + > + page->valid =3D VM_PAGE_BITS_ALL; > + > + return VM_PAGER_OK; > +} > + > +static struct cdev_pager_ops contigmem_cdev_pager_ops =3D { > + .cdev_pg_ctor =3D contigmem_cdev_pager_ctor, > + .cdev_pg_dtor =3D contigmem_cdev_pager_dtor, > + .cdev_pg_fault =3D contigmem_cdev_pager_fault, }; > + > static int > contigmem_mmap_single(struct cdev *cdev, vm_ooffset_t *offset, > vm_size_t size, > struct vm_object **obj, int nprot) > { > + struct contigmem_vm_handle *vmh; > uint64_t buffer_index; >=20 > /* > @@ -238,10 +362,17 @@ contigmem_mmap_single(struct cdev *cdev, > vm_ooffset_t *offset, vm_size_t size, > if (buffer_index >=3D contigmem_num_buffers) > return EINVAL; >=20 > - memset(contigmem_buffers[buffer_index], 0, > contigmem_buffer_size); > - *offset =3D > (vm_ooffset_t)vtophys(contigmem_buffers[buffer_index]); > - *obj =3D vm_pager_allocate(OBJT_DEVICE, cdev, size, nprot, *offset, > - curthread->td_ucred); > + if (size > contigmem_buffer_size) > + return EINVAL; > + > + vmh =3D malloc(sizeof(*vmh), M_CONTIGMEM, M_NOWAIT | > M_ZERO); > + if (vmh =3D=3D NULL) > + return ENOMEM; > + vmh->buffer_index =3D buffer_index; > + > + *offset =3D > (vm_ooffset_t)vtophys(contigmem_buffers[buffer_index].addr); > + *obj =3D cdev_pager_allocate(vmh, OBJT_DEVICE, > &contigmem_cdev_pager_ops, > + size, nprot, *offset, curthread->td_ucred); >=20 > return 0; > } > -- > 2.12.1