Re: Integration of SCST in the mainstream Linux kernel
On Mon, 2008-02-04 at 22:43 +, Alan Cox wrote: better. So for example, I personally suspect that ATA-over-ethernet is way better than some crazy SCSI-over-TCP crap, but I'm biased for simple and low-level, and against those crazy SCSI people to begin with. Current ATAoE isn't. It can't support NCQ. A variant that did NCQ and IP would probably trash iSCSI for latency if nothing else. But ATAoE is boring because it's not IP. Which means no routing, firewalls, tunnels, congestion control, etc. NBD and iSCSI (for all its hideous growths) can take advantage of these things. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH rc8-mm1] hotfix libata-scsi corruption
On Tue, 2008-01-22 at 22:59 +, Hugh Dickins wrote: On Tue, 22 Jan 2008, James Bottomley wrote: libsas looks to be OK because it specifically kmallocs a 512 byte buffer which should (for off slab data) be 512 byte aligned. I don't remember the various SLAB and SLOB and SLUB rules offhand: I'm not sure it's safe to rely on such alignment on all of them It doesn't work that way with SLOB kmalloc (nor did it in pre-slabified kmalloc). One shouldn't be surprised if a SLAB/SLUB debugging feature breaks that alignment either. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: QUEUE_FLAG_CLUSTER: not working in 2.6.24 ?
On Mon, Dec 17, 2007 at 11:24:57AM -0800, Randy Dunlap wrote: On Sun, 16 Dec 2007 21:55:20 + Mel Gorman wrote: Just using cp to read the file is enough to cause problems but I included a very basic program below that produces the BUG_ON checks. Is this a known issue or am I using the interface incorrectly? I'd say you're using it correctly but you've found a hitherto unknown bug. On i386 highmem machines with CONFIG_HIGHPTE (at least) pte_offset_map() takes kmap_atomic(), so pagemap_pte_range() can't do copy_to_user() as it presently does. Drat. Still, that shouldn't really disrupt the testing which you're doing. You could disable CONFIG_HIGHPTE to shut it up. Yes, that did the trick. Using pagemap, it was trivial to show that the 2.6.24-rc5-mm1 kernel was placing pages in reverse physical order like the following output shows b: 32763 v: 753091 p:65559 . 65558 contig: 1 b: 32764 v: 753092 p:65558 . 65557 contig: 1 b: 32765 v: 753093 p:65557 . 65556 contig: 1 b: 32766 v: 753094 p:65556 . 6 contig: 1 b: 32767 v: 753095 p:6 . 6 contig: 1 p: is the PFN of the page v: is the page offset within an anonymous mapping and b: is the number of non-contiguous blocks in the anonymous mapping. With the patch applied, it looks more like; b: 1232 v: 752964 p:58944 87328 contig: 15 b: 1233 v: 752980 p:87328 91200 contig: 15 b: 1234 v: 752996 p:91200 40272 contig: 15 b: 1235 v: 753012 p:40272 85664 contig: 15 b: 1236 v: 753028 p:85664 87312 contig: 15 so mappings are using contiguous pages again. This was the final test program I used in case it's of any interest. Thanks /* * showcontiguous.c * * Use the /proc/pid/pagemap interface to give an indication of how contiguous * physical memory is in an anonymous virtual memory mapping */ Matt, Did you ever make your python pagemap scripts available? If not, would you? There's a collection of them at http://selenic.com/repo/pagemap. They're largely proof of concept, and I'm not sure I finished adapting them all to the final 64-bit interface. As it happens, the above regression I actually spotted immediately by doing a simple hexdump on my very first test of the interface - lots of pfns counting backwards. Mentioned it a few times to various people in the cc: list and on lkml but never got around to tracking it down myself.. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: QUEUE_FLAG_CLUSTER: not working in 2.6.24 ?
On Fri, Dec 14, 2007 at 06:02:06PM -0800, Andrew Morton wrote: On Sat, 15 Dec 2007 01:09:41 + Mel Gorman [EMAIL PROTECTED] wrote: On (13/12/07 14:29), Andrew Morton didst pronounce: The simple way seems to be to malloc a large area, touch every page and then look at the physical pages assigned ... they now mostly seem to be descending in physical address. OIC. -mm's /proc/pid/pagemap can be used to get the pfn's... I tried using pagemap to verify the patch but it triggered BUG_ON checks. Perhaps I am using the interface wrong but I would still not expect it to break in this fashion. I tried 2.6.24-rc4-mm1, 2.6.24-rc5-mm1, 2.6.24-rc5 with just the maps4 patches applied and 2.6.23 with maps4 patches applied. Each time I get errors like this; [ 90.108315] BUG: sleeping function called from invalid context at include/asm/uaccess_32.h:457 [ 90.211227] in_atomic():1, irqs_disabled():0 [ 90.262251] no locks held by showcontiguous/2814. [ 90.318475] Pid: 2814, comm: showcontiguous Not tainted 2.6.24-rc5 #1 [ 90.395344] [c010522a] show_trace_log_lvl+0x1a/0x30 [ 90.456948] [c0105bb2] show_trace+0x12/0x20 [ 90.510173] [c0105eee] dump_stack+0x6e/0x80 [ 90.563409] [c01205b3] __might_sleep+0xc3/0xe0 [ 90.619765] [c02264fd] copy_to_user+0x3d/0x60 [ 90.675153] [c01b3e9c] add_to_pagemap+0x5c/0x80 [ 90.732513] [c01b43e8] pagemap_pte_range+0x68/0xb0 [ 90.793010] [c0175ed2] walk_page_range+0x112/0x210 [ 90.853482] [c01b47c6] pagemap_read+0x176/0x220 [ 90.910863] [c0182dc4] vfs_read+0x94/0x150 [ 90.963058] [c01832fd] sys_read+0x3d/0x70 [ 91.014219] [c0104262] syscall_call+0x7/0xb ... Just using cp to read the file is enough to cause problems but I included a very basic program below that produces the BUG_ON checks. Is this a known issue or am I using the interface incorrectly? I'd say you're using it correctly but you've found a hitherto unknown bug. On i386 highmem machines with CONFIG_HIGHPTE (at least) pte_offset_map() takes kmap_atomic(), so pagemap_pte_range() can't do copy_to_user() as it presently does. Damn, I coulda sworn I fixed that. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi_lib.c: continue after MEDIUM_ERROR
On Fri, Feb 02, 2007 at 11:06:19AM -0500, Mark Lord wrote: Alan wrote: If this is the right strategy for disk recovery for a given type of device then this ought to be an automatic strategy. Most end users will not have the knowledge to frob about in sysfs, and if the bad sector hits at the wrong moment a sensible automatic recovery strategy is going to do the right thing faster than human intervention, which may be some hours away. I think something we seem to be discussing here are the opposite aims of enterprise RAID (traditional SCSI market) versus desktop filesystems (now the number one user of Linux SCSI, courtesy of libata). With RAID, it's safe to suggest that a very fast, bounded exit from EH is almost always what the customer / end-user wants, because the higher level RAID management can then deal with the failed drive offline. But for a desktop filesystem, failing out quickly and bulk-failing megabytes around a couple of bad sectors is not good -- no redundancy, and this will lead to unneeded data loss. It's beginning to look like this needs to be run-time tuneable, per block minor device (per-partition), through sysfs at a minimum. The RAID tools could then automatically choose settings to bias more towards an instant exit when errors are found, whereas a non-RAID desktop could select a more reliable recovery strategy. Right now, with Jame's patch (earlier in this thread), the whole scheme is heavily weighted towards the RAID instant exit strategy, which is making me quite nervous about the data on my laptop. Also worth considering is that spending minutes trying to reread damaged sectors is likely to accelerate your death spiral. More data may be recoverable if you give up quickly in a first pass, then go back and manually retry damaged bits with smaller I/Os. Another approach is to have separate retries per hard sector and hard errors per MB at the block device level. We don't want to have the same number of retries for a 64KB block as a 1MB block and we certainly don't want to do 2K retries in a row. So for instance, I could have the first set to 1 and the second set to 16. For a 256KB read, this gets scaled down to 4, which means we'll retry each sector once and fail the whole I/O when we hit the 5th sector error. More reasonable number might be 0 and 1, meaning don't do OS level retries on sectors and fail the whole I/O on the second sector error (or immediately for smaller reads). It might also be informative to add a kernel message reporting if a retry (in the non-transient cases) ever actually succeeds. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi_lib.c: continue after MEDIUM_ERROR
On Fri, Feb 02, 2007 at 05:58:04PM -0500, Mark Lord wrote: Matt Mackall wrote: .. Also worth considering is that spending minutes trying to reread damaged sectors is likely to accelerate your death spiral. More data may be recoverable if you give up quickly in a first pass, then go back and manually retry damaged bits with smaller I/Os. All good input. But what was being debated here is not so much the retrying of known-bad sectors, but rather what to do about the kiBs or MiBs of sectors remaining in a merged request after hitting a single bad sector mid-way. Yep, that's precisely what was addressed in the part you snipped. My main point being that what to do about the remaining workload should be dependent on the size of the I/O. If we encounter errors on sectors 4,5,6,7,8.. of a 1MB request, we should have a threshold for giving up. It's not unreasonable for that threshold to be larger than 1, but it should not be 2048. And if we do the I/O as four 256KB requests, we should have approximately the same number of retries (assuming the whole region's bad). -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] compile out scsi_ioctl when no SCSI/IDE/etc. (take 3)
On Mon, Apr 11, 2005 at 05:50:08PM +0200, Fillod Stephane wrote: Hi Matt and everyone, I fixed again my previous patch (take2): * CDROM_PKTCDVD draws dependency * removing SCSI_IOCTL from displayed options, as suggested by Milton Miller. * rewrote ifdefs in the middle of ide.c functions as dummy function in header, as suggested by Matthew Wilcox. Matt, how do we proceed from here? If the patch is to your liking, can I hand it off to you, for eventual submission to Linus or the SCSI team? Since this is already cc:ed to linux-scsi, hopefully they'll pick it up. But this current version looks good to me. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE 0/6] Open-iSCSI High-Performance Initiator for Linux
On Wed, Mar 09, 2005 at 11:29:03AM -0800, Bryan Henderson wrote: I'd like to remind everyone that user space processes aside, there's still a basic ISCSI initiator resource inversion that needs to be fixed to avoid deadlock: ISCSI initiator driver sits below the memory pool (I..e a pageout involving ISCSI might be prerequisite to getting memory) Socket layer sits below ISCSI (ISCSI initiator driver relies on socket services) Memory pool sits below socket layer (socket layer allocates memory from the main pool) One might say as long as that's there, close is good enough on the user space initiator component. This issue is becoming increasingly pressing and hopefully there'll be a plan for addressing it soon. Unfortunately, none of the proposed solutions help the userspace inversion problem. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE 0/6] Open-iSCSI High-Performance Initiator for Linux
On Sun, Mar 06, 2005 at 11:03:14PM -0800, Alex Aizman wrote: As far as user/kernel, the existing iSCSI initiators bloat the kernel with ever-growing control plane code, including but not limited to: iSCSI discovery, Login (Authentication and Operational), session and connection management, connection-level error processing, iSCSI Text, Nop-Out/In, Async Message, iSNS, SLP, Radius... Open-iSCSI puts the entire control plane in the user space. This control plane talks to the data plane via well defined interface over the netlink transport. How big is the userspace client? How does this perform under memory pressure? If the userspace iSCSI client is paged out for whatever reason, and flushing _to_ an iSCSI device is necessary to page the usersace portion back in, and the connection needs restarting or the like to flush... Performance. This is the major goal and motivation for this project. As it happens, iSCSI has to compete with Fibre Channel, which is a more entrenched technology in the storage space. In addition, the soft iSCSI implementation have to show good results in presence of specialized hardware offloads. Our today's performance numbers are: - 450MB/sec Read on a single connection (2-way 2.4Ghz Opteron, 64KB block size); With what network hardware and drives, please? -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE 0/6] Open-iSCSI High-Performance Initiator for Linux
On Tue, Mar 08, 2005 at 09:51:39PM -0800, Alex Aizman wrote: Matt Mackall wrote: How big is the userspace client? Hmm.. x86 executable? source? Anyway, there's about 12,000 lines of user space code, and growing. In the kernel we have approx. 3,300 lines. - 450MB/sec Read on a single connection (2-way 2.4Ghz Opteron, 64KB block size); With what network hardware and drives, please? Neterion's 10GbE adapters. RAM disk on the target side. Ahh. Snipped my question about userspace deadlocks - that was the important one. It is in fact why the sfnet one is written as it is - it originally had a userspace component and turned out to be easy to deadlock under load because of it. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE 1/6] Open-iSCSI High-Performance Initiator for Linux
On Sun, Mar 06, 2005 at 11:04:51PM -0800, Alex Aizman wrote:
SCSI LLDD consists of 3 files:
- iscsi_if.c (iSCSI open interface over netlink);
- iscsi_tcp.[ch] (iSCSI transport over TCP/IP).
Signed-off-by: Alex Aizman [EMAIL PROTECTED]
Signed-off-by: Dmitry Yusupov [EMAIL PROTECTED]
Some minor comments..
+int
+iscsi_control_recv_pdu(iscsi_cnx_h cp_cnx, struct iscsi_hdr *hdr,
+ char *data, uint32_t data_size)
Return value on the same line as the function name, please.
+{
+ struct nlmsghdr *nlh;
+ struct sk_buff *skb;
Tabs except at the beginning of the line are a nuisance.
+ if (!skb) {
+ return -ENOMEM;
+ }
Drop the braces around single statements.
+EXPORT_SYMBOL_GPL(iscsi_control_recv_pdu);
Is this more than one module?
+iscsi_control_cnx_error(iscsi_cnx_h cp_cnx, iscsi_err_e error)
Your type-naming scheme is a little unusual for the kernel. err_e
appears redundant. And _h for handle could simply be _t for (opaque)
type.
+static void
+iscsi_if_rx(struct sock *sk, int len)
+{
+ struct sk_buff *skb;
+ while ((skb = skb_dequeue(sk-sk_receive_queue)) != NULL) {
Assignments in tests are somewhat discouraged and !f is preferred to f
!= NULL.
+#ifndef DEBUG_ASSERT
+#ifdef BUG_ON
+#undef BUG_ON
+#endif
+#define BUG_ON(expr)
+#endif
Don't do that, please. An assertion that's not enabled is worse than
no assertion at all.
+static inline void
+iscsi_buf_init_hdr(struct iscsi_conn *conn, struct iscsi_buf *ibuf,
+char *vbuf, u8 *crc)
+{
+ iscsi_buf_init_virt(ibuf, vbuf, sizeof(struct iscsi_hdr));
+ if (conn-hdrdgst_en) {
+ crypto_digest_init(conn-tx_tfm);
+ crypto_digest_update(conn-tx_tfm, ibuf-sg, 1);
+ crypto_digest_final(conn-tx_tfm, crc);
I believe you'll find that crypto_digest_digest does that all for you.
+#define iscsi_conn_get(rdd) (struct iscsi_conn*)(rdd)-arg.data
+#define iscsi_conn_set(rdd, conn) (rdd)-arg.data = conn
Inlines, please. The second one is slightly broken without parens.
+ * PDU header scattered accross SKB's,
across
+ switch(conn-in.opcode) {
+ case ISCSI_OP_SCSI_CMD_RSP:
This switch looks like it could happily be in another function. The
indentation is making it cramped.
+/*
+ * iscsi_ctask_copy - copy skb bits to the destanation cmd task
+ *
+ * The function calls skb_copy_bits() and updates per-connection and
+ * per-cmd byte counters.
+ */
+static inline int
+iscsi_ctask_copy(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask,
+ void *buf, int buf_size)
+{
Almost kerneldoc style, but not quite.
+ BUG_ON(ctask != (void*)sc-SCp.ptr);
Strange cast here.
+ if (sc-use_sg) {
+ int i;
Mixed tabs and spaces.
+ default:
+ BUG_ON(1);
BUG()?
+static void
+iscsi_tcp_data_ready(struct sock *sk, int flag)
+{
+ struct iscsi_conn *conn = (struct iscsi_conn*)sk-sk_user_data;
Redundant cast here.
+static void
+iscsi_write_space(struct sock *sk)
+{
+ struct iscsi_conn *conn = (struct iscsi_conn*)sk-sk_user_data;
+ conn-old_write_space(sk);
+ debug_tcp(iscsi_write_space: cid %d\n, conn-id);
+ conn-suspend = 0; wmb();
Sneaky. Separate line, please, and maybe a comment as to why the
barrier is needed.
+ debug_tcp(sendhdr %lx %d bytes at offset %d sent %d res %d\n,
+ (long)page_address(buf-sg.page), size, offset, buf-sent, res);
%p for page_address?
+static inline int
+iscsi_sendpage(struct iscsi_conn *conn, struct iscsi_buf *buf,
+int *count, int *sent)
+{
+ ssize_t (*sendpage)(struct socket *, struct page *, int, size_t, int);
[...]
+ sendpage = sk-ops-sendpage ? : sock_no_sendpage;
+
+ res = sendpage(sk, buf-sg.page, offset, size, flags);
Can't we just do an if (a) a() else b() here? The ? empty : syntax
is nonstandard.
+iscsi_solicit_data_cont(struct iscsi_conn *conn, struct iscsi_cmd_task
*ctask,
+ struct iscsi_r2t_info *r2t, int left)
+{
+ struct iscsi_data *hdr;
+ struct iscsi_data_task *dtask;
+ struct scsi_cmnd *sc = ctask-sc;
+ int new_offset;
+
+ dtask = mempool_alloc(ctask-datapool, GFP_ATOMIC);
+ hdr = dtask-hdr;
+ hdr-flags = 0;
+ hdr-rsvd2[0] = hdr-rsvd2[1] = hdr-rsvd3 =
+ hdr-rsvd4 = hdr-rsvd5 = hdr-rsvd6 = 0;
That looks odd, memset?
+static void
+iscsi_unsolicit_data_init(struct iscsi_conn *conn, struct iscsi_cmd_task
*ctask)
+{
+ struct iscsi_data *hdr;
+ struct iscsi_data_task *dtask;
+
+ dtask = mempool_alloc(ctask-datapool, GFP_ATOMIC);
+ hdr = dtask-hdr;
+ hdr-rsvd2[0] = hdr-rsvd2[1] = hdr-rsvd3 =
+ hdr-rsvd4 = hdr-rsvd5 = hdr-rsvd6 = 0;
And that looks familiar..
...
Boy howdy, this patch goes on for ever. Giving up at 9%.
--
Re: [ANNOUNCE 2/6] Open-iSCSI High-Performance Initiator for Linux
On Sun, Mar 06, 2005 at 11:12:06PM -0800, Alex Aizman wrote:
+#define iscsi_ptr(_handle) ((void*)(unsigned long)_handle)
+#define iscsi_handle(_ptr) ((uint64_t)(unsigned long)_ptr)
This is a bit wonky. Why is there a distinction?
+#ifndef ISCSI_PROTO_H
+#define ISCSI_PROTO_H
+
+#define ISCSI_VERSION_STR0.1.0
+#define ISCSI_DATE_STR 17-Jan-2005
+#define ISCSI_DRAFT20_VERSION0x00
+
+/* default iSCSI listen port for incoming connections */
+#define ISCSI_LISTEN_PORT3260
+
+/* Padding word length */
+#define PAD_WORD_LEN 4
Namespace.
+
+/*
+ * useful common(control and data pathes) macro
+ */
+#define ntoh24(p) (((p)[0] 16) | ((p)[1] 8) | ((p)[2]))
+#define hton24(p, v) { \
+p[0] = (((v) 16) 0xFF); \
+p[1] = (((v) 8) 0xFF); \
+p[2] = ((v) 0xFF); \
+}
+#define zero_data(p) {p[0]=0;p[1]=0;p[2]=0;}
These are specific to dlength, yes? Can we instead roll dlength and
hlength together, and do this with masking?
#define iscsi_hlen(hdr) (ntohl(hdr-hdlen)24)
#define iscsi_dlen(hdr) (ntohl(hdr-hdlen) 0xff)
#define iscsi_set_hlen(hdr, len) (hdr-hdlen=htonl(iscsi_dlen(hdr)|(len24)))
#define iscsi_set_hlen(hdr, len) (hdr-hdlen=htonl(len|(iscsi_hlen(hdr)24)))
The last two obviously have multiple evaluation, but you get the idea.
+/* SNACK Header */
Mmm, snacks.
+/* Reason for Reject */
+#define CMD_BEFORE_LOGIN 1
+#define DATA_DIGEST_ERROR2
+#define DATA_SNACK_REJECT3
+#define ISCSI_PROTOCOL_ERROR 4
+#define CMD_NOT_SUPPORTED5
+#define IMM_CMD_REJECT 6
+#define TASK_IN_PROGRESS 7
+#define INVALID_SNACK8
+#define BOOKMARK_REJECTED9
+#define BOOKMARK_NO_RESOURCES10
+#define NEGOTIATION_RESET11
+
+/* Max. number of Key=Value pairs in a text message */
+#define MAX_KEY_VALUE_PAIRS 8192
+
+/* maximum length for text keys/values */
+#define KEY_MAXLEN 64
+#define VALUE_MAXLEN 255
+#define TARGET_NAME_MAXLEN VALUE_MAXLEN
+
+#define DEFAULT_MAX_RECV_DATA_SEGMENT_LENGTH 8192
Namespace.
--
Mathematics is the supreme nostalgia of our time.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE 6/6] Open-iSCSI High-Performance Initiator for Linux
On Sun, Mar 06, 2005 at 11:19:03PM -0800, Alex Aizman wrote: +The latest development release is available at: +http://www.open-iscsi.org I think a URL in Kconfig and the source is sufficient, as this requires a userspace component which is a better place to bundle the docs. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
