[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <48C8BC8C.8080704@cs.columbia.edu>
Date: Thu, 11 Sep 2008 02:37:00 -0400
From: Oren Laadan <orenl@...columbia.edu>
To: MinChan Kim <minchan.kim@...il.com>
CC: dave@...ux.vnet.ibm.com, arnd@...db.de, jeremy@...p.org,
linux-kernel@...r.kernel.org, containers@...ts.linux-foundation.org
Subject: Re: [RFC v4][PATCH 8/9] File descriprtors (dump)
MinChan Kim wrote:
> On Tue, Sep 9, 2008 at 4:42 PM, Oren Laadan <orenl@...columbia.edu> wrote:
[...]
>> +#define CR_DEFAULT_FDTABLE 256 /* an initial guess */
>> +
>> +/**
>> + * cr_scan_fds - scan file table and construct array of open fds
>> + * @files: files_struct pointer
>> + * @fdtable: (output) array of open fds
>> + * @return: the number of open fds found
>> + *
>> + * Allocates the file descriptors array (*fdtable), caller should free
>> + */
>> +int cr_scan_fds(struct files_struct *files, int **fdtable)
>> +{
>> + struct fdtable *fdt;
>> + int *fdlist;
>> + int i, n, max;
>> +
>> + n = 0;
>> + max = CR_DEFAULT_FDTABLE;
>
> max is read-only variable so that you don't need to declare local variable.
> You can use macro.
It's actually used below - track size in case of krealloc()
>
>> + fdlist = kmalloc(max * sizeof(*fdlist), GFP_KERNEL);
>> + if (!fdlist)
>> + return -ENOMEM;
>> +
>> + spin_lock(&files->file_lock);
>> + fdt = files_fdtable(files);
>> + for (i = 0; i < fdt->max_fds; i++) {
>> + if (!fcheck_files(files, i))
>> + continue;
>> + if (n == max) {
>> + /* fcheck_files() is safe with drop/re-acquire
>> + * of the lock, as it tests: fd < max_fds */
>> + spin_unlock(&files->file_lock);
>> + max *= 2;
>> + if (max < 0) { /* overflow ? */
>> + n = -EMFILE;
>> + goto out;
>> + }
>> + fdlist = krealloc(fdlist, max, GFP_KERNEL);
>> + if (!fdlist) {
>> + n = -ENOMEM;
>> + goto out;
>> + }
>> + spin_lock(&files->file_lock);
>> + }
>> + fdlist[n++] = i;
>> + }
>> + spin_unlock(&files->file_lock);
>> +
>> + *fdtable = fdlist;
>> + out:
>> + return n;
>> +}
[...]
>> +int cr_write_files(struct cr_ctx *ctx, struct task_struct *t)
>> +{
>> + struct cr_hdr h;
>> + struct cr_hdr_files *hh = cr_hbuf_get(ctx, sizeof(*hh));
>> + struct files_struct *files;
>> + int *fdtable;
>> + int nfds, n, ret;
>> +
>> + h.type = CR_HDR_FILES;
>> + h.len = sizeof(*hh);
>> + h.parent = task_pid_vnr(t);
>> +
>> + files = get_files_struct(t);
>> +
>> + hh->objref = 0; /* will be meaningful with multiple processes */
>> +
>> + nfds = cr_scan_fds(files, &fdtable);
>> + if (nfds < 0) {
>> + ret = nfds;
>> + goto out;
>> + }
>> +
>> + hh->nfds = nfds;
>> +
>> + ret = cr_write_obj(ctx, &h, hh);
>> + cr_hbuf_put(ctx, sizeof(*hh));
>> + if (ret < 0)
>> + goto clean;
>> +
>> + cr_debug("nfds %d\n", nfds);
>> + for (n = 0; n < nfds; n++) {
>> + ret = cr_write_fd_ent(ctx, files, n);
>
> I think your intention is not 'n' but 'fdtable[n]' in argument.
Oops ... yes. Thanks.
[...]
Oren.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists