From a83a71708a6c908cc89921678e741565f1d6fff2 Mon Sep 17 00:00:00 2001 From: adilger Date: Tue, 12 Mar 2002 00:01:26 +0000 Subject: [PATCH] Add echo_connect/echo_disconnect to do proper module refcounting (avoid oops if we unload module while we have an open connection to it). The echo_preprw and echo_commitrw methods now clean up properly on error. --- lustre/obdecho/echo.c | 109 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 85 insertions(+), 24 deletions(-) diff --git a/lustre/obdecho/echo.c b/lustre/obdecho/echo.c index f531191..2f1c644 100644 --- a/lustre/obdecho/echo.c +++ b/lustre/obdecho/echo.c @@ -33,6 +33,30 @@ static struct obdo OA; static obd_count GEN; static long echo_pages = 0; +int echo_connect (struct obd_conn *conn) +{ + int rc; + + MOD_INC_USE_COUNT; + rc = gen_connect(conn); + + if (rc) + MOD_DEC_USE_COUNT; + + return rc; +} + +int echo_disconnect(struct obd_conn *conn) +{ + int rc; + + rc = gen_disconnect(conn); + if (!rc) + MOD_DEC_USE_COUNT; + + return rc; +} + static int echo_getattr(struct obd_conn *conn, struct obdo *oa) { memcpy(oa, &OA, sizeof(*oa)); @@ -45,26 +69,30 @@ int echo_preprw(int cmd, struct obd_conn *conn, int objcount, struct obd_ioobj *obj, int niocount, struct niobuf *nb, struct niobuf *res) { + struct niobuf *r = res; int rc = 0; int i; ENTRY; + memset(res, 0, sizeof(*res) * niocount); - CDEBUG(D_CACHE, "%s %d obdos with %d IOs\n", + CDEBUG(D_PAGE, "%s %d obdos with %d IOs\n", cmd == OBD_BRW_READ ? "reading" : "writing", objcount, niocount); for (i = 0; i < objcount; i++, obj++) { int j; - for (j = 0 ; j < obj->ioo_bufcnt ; j++, nb++, res++) { + for (j = 0 ; j < obj->ioo_bufcnt ; j++, nb++, r++) { unsigned long address; address = get_zeroed_page(GFP_KERNEL); if (!address) { - /* FIXME: cleanup old pages on error */ - EXIT; + CERROR("can't get new page %d/%d for id %Ld\n", + j, obj->ioo_bufcnt, obj->ioo_id); rc = -ENOMEM; + EXIT; + goto preprw_cleanup; } echo_pages++; @@ -79,13 +107,32 @@ int echo_preprw(int cmd, struct obd_conn *conn, int objcount, } */ - res->addr = address; - res->offset = nb->offset; - res->page = virt_to_page(address); - res->len = PAGE_SIZE; + r->addr = address; + r->offset = nb->offset; + r->page = virt_to_page(address); + r->len = nb->len; // r->flags } } + CDEBUG(D_PAGE, "%ld pages allocated after prep\n", echo_pages); + + EXIT; + return 0; + +preprw_cleanup: + /* It is possible that we would rather handle errors by allow + * any already-set-up pages to complete, rather than tearing them + * all down again. I believe that this is what the in-kernel + * prep/commit operations do. + */ + CERROR("cleaning up %d pages (%d obdos)\n", r - res, objcount); + while (r-- > res) { + unsigned long addr = r->addr; + + free_pages(addr, 0); + echo_pages--; + } + memset(res, 0, sizeof(*res) * niocount); return rc; } @@ -93,45 +140,59 @@ int echo_preprw(int cmd, struct obd_conn *conn, int objcount, int echo_commitrw(int cmd, struct obd_conn *conn, int objcount, struct obd_ioobj *obj, int niocount, struct niobuf *res) { - int i; + struct niobuf *r = res; int rc = 0; + int i; + ENTRY; - CDEBUG(D_CACHE, "%s %d obdos with %d IOs\n", + CDEBUG(D_PAGE, "%s %d obdos with %d IOs\n", cmd == OBD_BRW_READ ? "reading" : "writing", objcount, niocount); + if (niocount && !r) { + CERROR("NULL res niobuf with niocount %d\n", niocount); + EXIT; + return -EINVAL; + } + for (i = 0; i < objcount; i++, obj++) { int j; - for (j = 0 ; j < obj->ioo_bufcnt ; j++, res++) { + for (j = 0 ; j < obj->ioo_bufcnt ; j++, r++) { unsigned long addr; - if (!res) { - /* FIXME: cleanup remaining pages */ - CERROR("NULL buf, obj %Ld (%d), buf %d/%d\n", - obj->ioo_id, i, j, obj->ioo_bufcnt); - rc = -EINVAL; - } - - addr = res->addr; + addr = r->addr; if (!addr || !kern_addr_valid(addr)) { - /* FIXME: cleanup remaining pages */ CERROR("bad addr %lx, id %Ld (%d), buf %d/%d\n", addr, obj->ioo_id, i, j,obj->ioo_bufcnt); - rc = -EINVAL; + rc = -EFAULT; + EXIT; + goto commitrw_cleanup; } free_pages(addr, 0); echo_pages--; } } - CDEBUG(D_MALLOC, "%ld pages remain after commit\n", echo_pages); + CDEBUG(D_PAGE, "%ld pages remain after commit\n", echo_pages); + EXIT; + return 0; + +commitrw_cleanup: + CERROR("cleaning up %d pages (%d obdos)\n", niocount - (r - res) - 1, + objcount); + while (++r < res + niocount) { + unsigned long addr = r->addr; + + free_pages(addr, 0); + echo_pages--; + } return rc; } struct obd_ops echo_obd_ops = { - o_connect: gen_connect, - o_disconnect: gen_disconnect, + o_connect: echo_connect, + o_disconnect: echo_disconnect, o_getattr: echo_getattr, o_preprw: echo_preprw, o_commitrw: echo_commitrw, -- 1.8.3.1